From ce97312d109b21acb97d3ea243e214f20bd87cfc Mon Sep 17 00:00:00 2001 From: Arnaud Bienner Date: Wed, 31 May 2023 10:54:27 +0200 Subject: [PATCH] Implement BufferOverlap check for sprint/snprintf Differential Revision: https://reviews.llvm.org/D150430 --- .../lib/StaticAnalyzer/Checkers/CStringChecker.cpp | 53 ++++++++++++ clang/test/Analysis/buffer-overlap.c | 98 ++++++++++++++++++++++ 2 files changed, 151 insertions(+) create mode 100644 clang/test/Analysis/buffer-overlap.c diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 12b948a..01a3550 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -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" @@ -175,6 +176,8 @@ public: 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. @@ -228,6 +231,11 @@ public: 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 static assumeZero(CheckerContext &C, @@ -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. //===----------------------------------------------------------------------===// diff --git a/clang/test/Analysis/buffer-overlap.c b/clang/test/Analysis/buffer-overlap.c new file mode 100644 index 0000000..8414a76 --- /dev/null +++ b/clang/test/Analysis/buffer-overlap.c @@ -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 +} -- 2.7.4