Skip to content

Commit

Permalink
Implement BufferOverlap check for sprint/snprintf
Browse files Browse the repository at this point in the history
Differential Revision: https://reviews.llvm.org/D150430
  • Loading branch information
ArnaudBienner committed May 31, 2023
1 parent 0b42ee4 commit ce97312
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 0 deletions.
53 changes: 53 additions & 0 deletions clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//

#include "InterCheckerAPI.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/CharInfo.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
Expand Down Expand Up @@ -175,6 +176,8 @@ class CStringChecker : public Checker< eval::Call,
std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Regular)},
{{CDF_MaybeBuiltin, {"bzero"}, 2}, &CStringChecker::evalBzero},
{{CDF_MaybeBuiltin, {"explicit_bzero"}, 2}, &CStringChecker::evalBzero},
{{CDF_MaybeBuiltin, {"sprintf"}, 2}, &CStringChecker::evalSprintf},
{{CDF_MaybeBuiltin, {"snprintf"}, 2}, &CStringChecker::evalSnprintf},
};

// These require a bit of special handling.
Expand Down Expand Up @@ -228,6 +231,11 @@ class CStringChecker : public Checker< eval::Call,
void evalMemset(CheckerContext &C, const CallExpr *CE) const;
void evalBzero(CheckerContext &C, const CallExpr *CE) const;

void evalSprintf(CheckerContext &C, const CallExpr *CE) const;
void evalSnprintf(CheckerContext &C, const CallExpr *CE) const;
void evalSprintfCommon(CheckerContext &C, const CallExpr *CE, bool IsBounded,
bool IsBuiltin) const;

// Utility methods
std::pair<ProgramStateRef , ProgramStateRef >
static assumeZero(CheckerContext &C,
Expand Down Expand Up @@ -2352,6 +2360,51 @@ void CStringChecker::evalBzero(CheckerContext &C, const CallExpr *CE) const {
C.addTransition(State);
}

void CStringChecker::evalSprintf(CheckerContext &C, const CallExpr *CE) const {
CurrentFunctionDescription = "'sprintf'";
bool IsBI = CE->getBuiltinCallee() == Builtin::BI__builtin___sprintf_chk;
evalSprintfCommon(C, CE, /* IsBounded */ false, IsBI);
}

void CStringChecker::evalSnprintf(CheckerContext &C, const CallExpr *CE) const {
CurrentFunctionDescription = "'snprintf'";
bool IsBI = CE->getBuiltinCallee() == Builtin::BI__builtin___snprintf_chk;
evalSprintfCommon(C, CE, /* IsBounded */ true, IsBI);
}

void CStringChecker::evalSprintfCommon(CheckerContext &C, const CallExpr *CE,
bool IsBounded, bool IsBuiltin) const {
ProgramStateRef State = C.getState();
DestinationArgExpr Dest = {CE->getArg(0), 0};

const auto NumParams = CE->getCalleeDecl()->getAsFunction()->getNumParams();
assert(CE->getNumArgs() >= NumParams);

const auto AllArguments =
llvm::make_range(CE->getArgs(), CE->getArgs() + CE->getNumArgs());
const auto VariadicArguments = drop_begin(enumerate(AllArguments), NumParams);

for (const auto &[ArgIdx, ArgExpr] : VariadicArguments) {
// We consider only string buffers
if (const QualType type = ArgExpr->getType();
!type->isAnyPointerType() ||
!type->getPointeeType()->isAnyCharacterType())
continue;
SourceArgExpr Source = {ArgExpr, unsigned(ArgIdx)};

// Ensure the buffers do not overlap.
SizeArgExpr SrcExprAsSizeDummy = {Source.Expression, Source.ArgumentIndex};
State = CheckOverlap(
C, State,
(IsBounded ? SizeArgExpr{CE->getArg(1), 1} : SrcExprAsSizeDummy), Dest,
Source);
if (!State)
return;
}

C.addTransition(State);
}

//===----------------------------------------------------------------------===//
// The driver method, and other Checker callbacks.
//===----------------------------------------------------------------------===//
Expand Down
98 changes: 98 additions & 0 deletions clang/test/Analysis/buffer-overlap.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// RUN: %clang_analyze_cc1 -verify %s \
// RUN: -analyzer-checker=alpha.unix.cstring.BufferOverlap
//
// RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS \
// RUN: -analyzer-checker=alpha.unix.cstring.BufferOverlap
//
// RUN: %clang_analyze_cc1 -verify %s -DVARIANT \
// RUN: -analyzer-checker=alpha.unix.cstring.BufferOverlap
//
// RUN: %clang_analyze_cc1 -verify %s -DVARIANT -DUSE_BUILTINS \
// RUN: -analyzer-checker=alpha.unix.cstring.BufferOverlap

// This provides us with four possible sprintf() definitions.

#ifdef USE_BUILTINS
#define BUILTIN(f) __builtin_##f
#else /* USE_BUILTINS */
#define BUILTIN(f) f
#endif /* USE_BUILTINS */

typedef typeof(sizeof(int)) size_t;

#ifdef VARIANT

#define __sprintf_chk BUILTIN(__sprintf_chk)
#define __snprintf_chk BUILTIN(__snprintf_chk)
int __sprintf_chk (char * __restrict str, int flag, size_t os,
const char * __restrict fmt, ...);
int __snprintf_chk (char * __restrict str, size_t len, int flag, size_t os,
const char * __restrict fmt, ...);

#define sprintf(str, ...) __sprintf_chk(str, 0, __builtin_object_size(str, 0), __VA_ARGS__)
#define snprintf(str, len, ...) __snprintf_chk(str, len, 0, __builtin_object_size(str, 0), __VA_ARGS__)

#else /* VARIANT */

#define sprintf BUILTIN(sprintf)
int sprintf(char *restrict buffer, const char *restrict format, ... );
int snprintf(char *restrict buffer, size_t bufsz,
const char *restrict format, ... );
#endif /* VARIANT */

void test_sprintf1() {
char a[4] = {0};
sprintf(a, "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
}

void test_sprintf2() {
char a[4] = {0};
sprintf(a, "%s", a); // expected-warning{{Arguments must not be overlapping buffers}}
}

void test_sprintf3() {
char a[4] = {0};
sprintf(a, "%s/%s", a, a); // expected-warning{{Arguments must not be overlapping buffers}}
}

void test_sprintf4() {
char a[4] = {0};
sprintf(a, "%d", 42); // no-warning
}

void test_sprintf5() {
char a[4] = {0};
char b[4] = {0};
sprintf(a, "%s", b); // no-warning
}

void test_snprintf1() {
char a[4] = {0};
snprintf(a, sizeof(a), "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
}

void test_snprintf2() {
char a[4] = {0};
snprintf(a+1, sizeof(a)-1, "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
}

void test_snprintf3() {
char a[4] = {0};
snprintf(a, sizeof(a), "%s", a); // expected-warning{{Arguments must not be overlapping buffers}}
}

void test_snprintf4() {
char a[4] = {0};
snprintf(a, sizeof(a), "%s/%s", a, a); // expected-warning{{Arguments must not be overlapping buffers}}
}

void test_snprintf5() {
char a[4] = {0};
snprintf(a, sizeof(a), "%d", 42); // no-warning
}

void test_snprintf6() {
char a[4] = {0};
char b[4] = {0};
snprintf(a, sizeof(a), "%s", b); // no-warning
}

0 comments on commit ce97312

Please sign in to comment.