Skip to content

Commit

Permalink
[clang][analyzer] Model allocation behavior or getdelim/geline (#83138)
Browse files Browse the repository at this point in the history
`getdelim` and `getline` may free, allocate, or re-allocate the input
buffer, ensuring its size is enough to hold the incoming line, the
delimiter, and the null terminator.

`*lineptr` must be a valid argument to `free`, which means it can be
either

1. `NULL`, in which case these functions perform an allocation
equivalent to a call to `malloc` even on failure.
2. A pointer returned by the `malloc` family of functions. Other
pointers are UB (`alloca`, a pointer to a static, to a stack variable, etc.)
  • Loading branch information
alejandro-alvarez-sonarsource committed Mar 6, 2024
1 parent 6540f16 commit 67c6ad6
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H

#include "ProgramState_Fwd.h"
#include "SVals.h"

#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
#include "clang/Basic/OperatorKinds.h"
Expand Down Expand Up @@ -110,6 +113,9 @@ class OperatorKind {
OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
bool IsBinary);

std::optional<DefinedSVal> getPointeeDefVal(SVal PtrSVal,
ProgramStateRef State);

} // namespace ento

} // namespace clang
Expand Down
77 changes: 71 additions & 6 deletions clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,8 @@ class MallocChecker
CHECK_FN(checkGMemdup)
CHECK_FN(checkGMallocN)
CHECK_FN(checkGMallocN0)
CHECK_FN(preGetdelim)
CHECK_FN(checkGetdelim)
CHECK_FN(checkReallocN)
CHECK_FN(checkOwnershipAttr)

Expand All @@ -391,6 +393,11 @@ class MallocChecker
using CheckFn = std::function<void(const MallocChecker *,
const CallEvent &Call, CheckerContext &C)>;

const CallDescriptionMap<CheckFn> PreFnMap{
{{{"getline"}, 3}, &MallocChecker::preGetdelim},
{{{"getdelim"}, 4}, &MallocChecker::preGetdelim},
};

const CallDescriptionMap<CheckFn> FreeingMemFnMap{
{{{"free"}, 1}, &MallocChecker::checkFree},
{{{"if_freenameindex"}, 1}, &MallocChecker::checkIfFreeNameIndex},
Expand Down Expand Up @@ -439,6 +446,8 @@ class MallocChecker
std::bind(&MallocChecker::checkRealloc, _1, _2, _3, false)},
{{{"g_realloc_n"}, 3}, &MallocChecker::checkReallocN},
{{{"g_try_realloc_n"}, 3}, &MallocChecker::checkReallocN},
{{{"getline"}, 3}, &MallocChecker::checkGetdelim},
{{{"getdelim"}, 4}, &MallocChecker::checkGetdelim},
};

bool isMemCall(const CallEvent &Call) const;
Expand Down Expand Up @@ -588,11 +597,14 @@ class MallocChecker
/// }
/// \param [in] ReturnsNullOnFailure Whether the memory deallocation function
/// we're modeling returns with Null on failure.
/// \param [in] ArgValOpt Optional value to use for the argument instead of
/// the one obtained from ArgExpr.
/// \returns The ProgramState right after deallocation.
[[nodiscard]] ProgramStateRef
FreeMemAux(CheckerContext &C, const Expr *ArgExpr, const CallEvent &Call,
ProgramStateRef State, bool Hold, bool &IsKnownToBeAllocated,
AllocationFamily Family, bool ReturnsNullOnFailure = false) const;
AllocationFamily Family, bool ReturnsNullOnFailure = false,
std::optional<SVal> ArgValOpt = {}) const;

// TODO: Needs some refactoring, as all other deallocation modeling
// functions are suffering from out parameters and messy code due to how
Expand Down Expand Up @@ -1423,6 +1435,50 @@ void MallocChecker::checkGMallocN0(const CallEvent &Call,
C.addTransition(State);
}

void MallocChecker::preGetdelim(const CallEvent &Call,
CheckerContext &C) const {
if (!Call.isGlobalCFunction())
return;

ProgramStateRef State = C.getState();
const auto LinePtr = getPointeeDefVal(Call.getArgSVal(0), State);
if (!LinePtr)
return;

// FreeMemAux takes IsKnownToBeAllocated as an output parameter, and it will
// be true after the call if the symbol was registered by this checker.
// We do not need this value here, as FreeMemAux will take care
// of reporting any violation of the preconditions.
bool IsKnownToBeAllocated = false;
State = FreeMemAux(C, Call.getArgExpr(0), Call, State, false,
IsKnownToBeAllocated, AF_Malloc, false, LinePtr);
if (State)
C.addTransition(State);
}

void MallocChecker::checkGetdelim(const CallEvent &Call,
CheckerContext &C) const {
if (!Call.isGlobalCFunction())
return;

ProgramStateRef State = C.getState();
// Handle the post-conditions of getline and getdelim:
// Register the new conjured value as an allocated buffer.
const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
if (!CE)
return;

SValBuilder &SVB = C.getSValBuilder();

const auto LinePtr = getPointeeDefVal(Call.getArgSVal(0), State);
const auto Size = getPointeeDefVal(Call.getArgSVal(1), State);
if (!LinePtr || !Size || !LinePtr->getAsRegion())
return;

State = setDynamicExtent(State, LinePtr->getAsRegion(), *Size, SVB);
C.addTransition(MallocUpdateRefState(C, CE, State, AF_Malloc, *LinePtr));
}

void MallocChecker::checkReallocN(const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
Expand Down Expand Up @@ -1895,15 +1951,17 @@ static void printExpectedDeallocName(raw_ostream &os, AllocationFamily Family) {
}
}

ProgramStateRef MallocChecker::FreeMemAux(
CheckerContext &C, const Expr *ArgExpr, const CallEvent &Call,
ProgramStateRef State, bool Hold, bool &IsKnownToBeAllocated,
AllocationFamily Family, bool ReturnsNullOnFailure) const {
ProgramStateRef
MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
const CallEvent &Call, ProgramStateRef State,
bool Hold, bool &IsKnownToBeAllocated,
AllocationFamily Family, bool ReturnsNullOnFailure,
std::optional<SVal> ArgValOpt) const {

if (!State)
return nullptr;

SVal ArgVal = C.getSVal(ArgExpr);
SVal ArgVal = ArgValOpt.value_or(C.getSVal(ArgExpr));
if (!isa<DefinedOrUnknownSVal>(ArgVal))
return nullptr;
DefinedOrUnknownSVal location = ArgVal.castAs<DefinedOrUnknownSVal>();
Expand Down Expand Up @@ -2881,6 +2939,13 @@ void MallocChecker::checkPreCall(const CallEvent &Call,
return;
}

// We need to handle getline pre-conditions here before the pointed region
// gets invalidated by StreamChecker
if (const auto *PreFN = PreFnMap.lookup(Call)) {
(*PreFN)(this, Call, C);
return;
}

// We will check for double free in the post visit.
if (const AnyFunctionCall *FC = dyn_cast<AnyFunctionCall>(&Call)) {
const FunctionDecl *FD = FC->getDecl();
Expand Down
9 changes: 9 additions & 0 deletions clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include <optional>

namespace clang {
Expand Down Expand Up @@ -182,5 +183,13 @@ OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
}
}

std::optional<DefinedSVal> getPointeeDefVal(SVal PtrSVal,
ProgramStateRef State) {
if (const auto *Ptr = PtrSVal.getAsRegion()) {
return State->getSVal(Ptr).getAs<DefinedSVal>();
}
return std::nullopt;
}

} // namespace ento
} // namespace clang
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ typedef __typeof(sizeof(int)) size_t;
void *malloc(size_t);
void *calloc(size_t, size_t);
void free(void *);
void *alloca(size_t);


#if __OBJC__
Expand Down
95 changes: 95 additions & 0 deletions clang/test/Analysis/getline-alloc.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,debug.ExprInspection -verify %s

// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,alpha.unix,debug.ExprInspection -verify %s

#include "Inputs/system-header-simulator.h"
#include "Inputs/system-header-simulator-for-malloc.h"

void test_getline_null_buffer() {
FILE *F1 = tmpfile();
if (!F1)
return;
char *buffer = NULL;
size_t n = 0;
if (getline(&buffer, &n, F1) > 0) {
char c = buffer[0]; // ok
}
free(buffer);
fclose(F1);
}

void test_getline_malloc_buffer() {
FILE *F1 = tmpfile();
if (!F1)
return;

size_t n = 10;
char *buffer = malloc(n);
char *ptr = buffer;

ssize_t r = getdelim(&buffer, &n, '\r', F1);
// ptr may be dangling
free(ptr); // expected-warning {{Attempt to free released memory}}
free(buffer); // ok
fclose(F1);
}

void test_getline_alloca() {
FILE *F1 = tmpfile();
if (!F1)
return;
size_t n = 10;
char *buffer = alloca(n);
getline(&buffer, &n, F1); // expected-warning {{Memory allocated by alloca() should not be deallocated}}
fclose(F1);
}

void test_getline_invalid_ptr() {
FILE *F1 = tmpfile();
if (!F1)
return;
size_t n = 10;
char *buffer = (char*)test_getline_invalid_ptr;
getline(&buffer, &n, F1); // expected-warning {{Argument to getline() is the address of the function 'test_getline_invalid_ptr', which is not memory allocated by malloc()}}
fclose(F1);
}

void test_getline_leak() {
FILE *F1 = tmpfile();
if (!F1)
return;

char *buffer = NULL;
size_t n = 0;
ssize_t read;

while ((read = getline(&buffer, &n, F1)) != -1) {
printf("%s\n", buffer);
}

fclose(F1); // expected-warning {{Potential memory leak}}
}

void test_getline_stack() {
size_t n = 10;
char buffer[10];
char *ptr = buffer;

FILE *F1 = tmpfile();
if (!F1)
return;

getline(&ptr, &n, F1); // expected-warning {{Argument to getline() is the address of the local variable 'buffer', which is not memory allocated by malloc()}}
}

void test_getline_static() {
static size_t n = 10;
static char buffer[10];
char *ptr = buffer;

FILE *F1 = tmpfile();
if (!F1)
return;

getline(&ptr, &n, F1); // expected-warning {{Argument to getline() is the address of the static variable 'buffer', which is not memory allocated by malloc()}}
}

0 comments on commit 67c6ad6

Please sign in to comment.