diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -952,7 +952,7 @@ unix.cstring.NullArg (C) """"""""""""""""""""""""" Check for null pointers being passed as arguments to C string functions: -``strlen, strnlen, strcpy, strncpy, strcat, strncat, strcmp, strncmp, strcasecmp, strncasecmp``. +``strlen, strnlen, strcpy, strncpy, strcat, strncat, strcmp, strncmp, strcasecmp, strncasecmp, wcslen, wcsnlen``. .. code-block:: c @@ -2699,7 +2699,7 @@ alpha.unix.cstring.BufferOverlap (C) """""""""""""""""""""""""""""""""""" -Checks for overlap in two buffer arguments. Applies to: ``memcpy, mempcpy``. +Checks for overlap in two buffer arguments. Applies to: ``memcpy, mempcpy, wmemcpy``. .. code-block:: c @@ -2712,7 +2712,7 @@ alpha.unix.cstring.NotNullTerminated (C) """""""""""""""""""""""""""""""""""""""" -Check for arguments which are not null-terminated strings; applies to: ``strlen, strnlen, strcpy, strncpy, strcat, strncat``. +Check for arguments which are not null-terminated strings; applies to: ``strlen, strnlen, strcpy, strncpy, strcat, strncat, wcslen, wcsnlen``. .. code-block:: c @@ -2724,15 +2724,24 @@ alpha.unix.cstring.OutOfBounds (C) """""""""""""""""""""""""""""""""" -Check for out-of-bounds access in string functions; applies to:`` strncopy, strncat``. +Check for out-of-bounds access in string functions, such as: +``memcpy, bcopy, strcpy, strncpy, strcat, strncat, memmove, memcmp, memset`` and more. -This check also applies to string literals, except there is a known bug in that -the analyzer cannot detect embedded NULL characters. +This check also works with string literals, except there is a known bug in that +the analyzer cannot detect embedded NULL characters when determining the string length. .. code-block:: c - void test() { - int y = strlen((char *)&test); // warn + void test1() { + const char str[] = "Hello world"; + char buffer[] = "Hello world"; + memcpy(buffer, str, sizeof(str) + 1); // warn + } + + void test2() { + const char str[] = "Hello world"; + char buffer[] = "Helloworld"; + memcpy(buffer, str, sizeof(str)); // warn } .. _alpha-unix-cstring-UninitializedRead: diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -26,9 +26,11 @@ #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/raw_ostream.h" +#include using namespace clang; using namespace ento; +using namespace std::placeholders; namespace { struct AnyArgExpr { @@ -118,10 +120,14 @@ const LocationContext *LCtx, const CallEvent *Call) const; - typedef void (CStringChecker::*FnCheck)(CheckerContext &, - const CallExpr *) const; + using FnCheck = std::function; + CallDescriptionMap Callbacks = { - {{CDF_MaybeBuiltin, "memcpy", 3}, &CStringChecker::evalMemcpy}, + {{CDF_MaybeBuiltin, "memcpy", 3}, + std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, false)}, + {{CDF_MaybeBuiltin, "wmemcpy", 3}, + std::bind(&CStringChecker::evalMemcpy, _1, _2, _3, true)}, {{CDF_MaybeBuiltin, "mempcpy", 3}, &CStringChecker::evalMempcpy}, {{CDF_MaybeBuiltin, "memcmp", 3}, &CStringChecker::evalMemcmp}, {{CDF_MaybeBuiltin, "memmove", 3}, &CStringChecker::evalMemmove}, @@ -135,7 +141,9 @@ {{CDF_MaybeBuiltin, "strncat", 3}, &CStringChecker::evalStrncat}, {{CDF_MaybeBuiltin, "strlcat", 3}, &CStringChecker::evalStrlcat}, {{CDF_MaybeBuiltin, "strlen", 1}, &CStringChecker::evalstrLength}, + {{CDF_MaybeBuiltin, "wcslen", 1}, &CStringChecker::evalstrLength}, {{CDF_MaybeBuiltin, "strnlen", 2}, &CStringChecker::evalstrnLength}, + {{CDF_MaybeBuiltin, "wcsnlen", 2}, &CStringChecker::evalstrnLength}, {{CDF_MaybeBuiltin, "strcmp", 2}, &CStringChecker::evalStrcmp}, {{CDF_MaybeBuiltin, "strncmp", 3}, &CStringChecker::evalStrncmp}, {{CDF_MaybeBuiltin, "strcasecmp", 2}, &CStringChecker::evalStrcasecmp}, @@ -152,14 +160,14 @@ StdCopyBackward{{"std", "copy_backward"}, 3}; FnCheck identifyCall(const CallEvent &Call, CheckerContext &C) const; - void evalMemcpy(CheckerContext &C, const CallExpr *CE) const; + void evalMemcpy(CheckerContext &C, const CallExpr *CE, bool IsWide) const; void evalMempcpy(CheckerContext &C, const CallExpr *CE) const; void evalMemmove(CheckerContext &C, const CallExpr *CE) const; void evalBcopy(CheckerContext &C, const CallExpr *CE) const; void evalCopyCommon(CheckerContext &C, const CallExpr *CE, ProgramStateRef state, SizeArgExpr Size, DestinationArgExpr Dest, SourceArgExpr Source, - bool Restricted, bool IsMempcpy) const; + bool Restricted, bool IsMempcpy, bool IsWide) const; void evalMemcmp(CheckerContext &C, const CallExpr *CE) const; @@ -240,10 +248,11 @@ AnyArgExpr Arg, SVal l) const; ProgramStateRef CheckLocation(CheckerContext &C, ProgramStateRef state, AnyArgExpr Buffer, SVal Element, - AccessKind Access) const; + AccessKind Access, bool IsWide = false) const; ProgramStateRef CheckBufferAccess(CheckerContext &C, ProgramStateRef State, AnyArgExpr Buffer, SizeArgExpr Size, - AccessKind Access) const; + AccessKind Access, + bool IsWide = false) const; ProgramStateRef CheckOverlap(CheckerContext &C, ProgramStateRef state, SizeArgExpr Size, AnyArgExpr First, AnyArgExpr Second) const; @@ -329,7 +338,8 @@ ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C, ProgramStateRef state, AnyArgExpr Buffer, SVal Element, - AccessKind Access) const { + AccessKind Access, + bool IsWide) const { // If a previous check has failed, propagate the failure. if (!state) @@ -344,17 +354,36 @@ if (!ER) return state; - if (ER->getValueType() != C.getASTContext().CharTy) - return state; + SValBuilder &svalBuilder = C.getSValBuilder(); + ASTContext &Ctx = svalBuilder.getContext(); + + // Get the index of the accessed element. + NonLoc Idx = ER->getIndex(); + + if (!IsWide) { + if (ER->getValueType() != Ctx.CharTy) + return state; + } else { + if (ER->getValueType() != Ctx.WideCharTy) + return state; + + QualType SizeTy = Ctx.getSizeType(); + NonLoc WideSize = + svalBuilder + .makeIntVal(Ctx.getTypeSizeInChars(Ctx.WideCharTy).getQuantity(), + SizeTy) + .castAs(); + SVal Offset = svalBuilder.evalBinOpNN(state, BO_Mul, Idx, WideSize, SizeTy); + if (Offset.isUnknown()) + return state; + Idx = Offset.castAs(); + } // Get the size of the array. const auto *superReg = cast(ER->getSuperRegion()); DefinedOrUnknownSVal Size = getDynamicExtent(state, superReg, C.getSValBuilder()); - // Get the index of the accessed element. - DefinedOrUnknownSVal Idx = ER->getIndex().castAs(); - ProgramStateRef StInBound, StOutBound; std::tie(StInBound, StOutBound) = state->assumeInBoundDual(Idx, Size); if (StOutBound && !StInBound) { @@ -385,11 +414,10 @@ return StInBound; } -ProgramStateRef CStringChecker::CheckBufferAccess(CheckerContext &C, - ProgramStateRef State, - AnyArgExpr Buffer, - SizeArgExpr Size, - AccessKind Access) const { +ProgramStateRef +CStringChecker::CheckBufferAccess(CheckerContext &C, ProgramStateRef State, + AnyArgExpr Buffer, SizeArgExpr Size, + AccessKind Access, bool IsWide) const { // If a previous check has failed, propagate the failure. if (!State) return nullptr; @@ -398,7 +426,7 @@ ASTContext &Ctx = svalBuilder.getContext(); QualType SizeTy = Size.Expression->getType(); - QualType PtrTy = Ctx.getPointerType(Ctx.CharTy); + QualType PtrTy = Ctx.getPointerType(IsWide ? Ctx.WideCharTy : Ctx.CharTy); // Check that the first buffer is non-null. SVal BufVal = C.getSVal(Buffer.Expression); @@ -432,7 +460,7 @@ SVal BufEnd = svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy); - State = CheckLocation(C, State, Buffer, BufEnd, Access); + State = CheckLocation(C, State, Buffer, BufEnd, Access, IsWide); // If the buffer isn't large enough, abort. if (!State) @@ -1161,7 +1189,7 @@ ProgramStateRef state, SizeArgExpr Size, DestinationArgExpr Dest, SourceArgExpr Source, bool Restricted, - bool IsMempcpy) const { + bool IsMempcpy, bool IsWide) const { CurrentFunctionDescription = "memory copy function"; // See if the size argument is zero. @@ -1204,8 +1232,8 @@ return; // Ensure the accesses are valid and that the buffers do not overlap. - state = CheckBufferAccess(C, state, Dest, Size, AccessKind::write); - state = CheckBufferAccess(C, state, Source, Size, AccessKind::read); + state = CheckBufferAccess(C, state, Dest, Size, AccessKind::write, IsWide); + state = CheckBufferAccess(C, state, Source, Size, AccessKind::read, IsWide); if (Restricted) state = CheckOverlap(C, state, Size, Dest, Source); @@ -1258,7 +1286,8 @@ } } -void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE) const { +void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE, + bool IsWide) const { // void *memcpy(void *restrict dst, const void *restrict src, size_t n); // The return value is the address of the destination buffer. DestinationArgExpr Dest = {CE->getArg(0), 0}; @@ -1269,7 +1298,8 @@ constexpr bool IsRestricted = true; constexpr bool IsMempcpy = false; - evalCopyCommon(C, CE, State, Size, Dest, Src, IsRestricted, IsMempcpy); + evalCopyCommon(C, CE, State, Size, Dest, Src, IsRestricted, IsMempcpy, + IsWide); } void CStringChecker::evalMempcpy(CheckerContext &C, const CallExpr *CE) const { @@ -1281,7 +1311,8 @@ constexpr bool IsRestricted = true; constexpr bool IsMempcpy = true; - evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy); + evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy, + false); } void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE) const { @@ -1293,7 +1324,8 @@ constexpr bool IsRestricted = false; constexpr bool IsMempcpy = false; - evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy); + evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy, + false); } void CStringChecker::evalBcopy(CheckerContext &C, const CallExpr *CE) const { @@ -1304,7 +1336,8 @@ constexpr bool IsRestricted = false; constexpr bool IsMempcpy = false; - evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy); + evalCopyCommon(C, CE, C.getState(), Size, Dest, Src, IsRestricted, IsMempcpy, + false); } void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE) const { @@ -2336,7 +2369,7 @@ // Check and evaluate the call. const auto *CE = cast(Call.getOriginExpr()); - (this->*Callback)(C, CE); + Callback(this, C, CE); // If the evaluate call resulted in no change, chain to the next eval call // handler. diff --git a/clang/test/Analysis/cstring.c b/clang/test/Analysis/cstring.c new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/cstring.c @@ -0,0 +1,126 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring \ +// RUN: -analyzer-checker=alpha.unix.cstring \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -w -verify %s + +typedef __SIZE_TYPE__ size_t; +typedef __WCHAR_TYPE__ wchar_t; +void *memcpy(void *dest, const void *src, + size_t count); +wchar_t *wmemcpy(wchar_t *restrict dest, const wchar_t *restrict src, + size_t count); +size_t strlen(const char *s); +size_t wcslen(const wchar_t *s); +size_t strnlen(const char *s, size_t maxlen); +size_t wcsnlen(const wchar_t *s, size_t maxlen); + +static const char str[] = "Hello world"; +static const wchar_t w_str[] = L"Hello world"; + +extern void clang_analyzer_eval(int); + +void test_sizeof(void) { + char buffer[32]; + memcpy(buffer, str, sizeof(str)); + memcpy(buffer, str, sizeof(str) + 1); // expected-warning {{Memory copy function accesses out-of-bound array element [alpha.unix.cstring.OutOfBounds]}} +} + +void w_test_sizeof(void) { + wchar_t w_buffer[32]; + wmemcpy(w_buffer, w_str, sizeof(w_str) / sizeof(w_str[0])); + wmemcpy(w_buffer, w_str, (sizeof(w_str) / sizeof(w_str[0])) + 1); // expected-warning {{Memory copy function accesses out-of-bound array element}} +} + +void test_strlen(void) { + char buffer[32]; + // FIXME: This should work with 'str' instead of 'str1' + const char str1[] = "Hello world"; + memcpy(buffer, str1, strlen(str1) + 1); + memcpy(buffer, str1, strlen(str1) + 2); // expected-warning {{Memory copy function accesses out-of-bound array element}} +} + +void w_test_wcslen(void) { + wchar_t w_buffer[32]; + // FIXME: This should work with 'w_str' instead of 'w_str1' + const wchar_t w_str1[] = L"Hello world"; + wmemcpy(w_buffer, w_str1, wcslen(w_str1) + 1); + wmemcpy(w_buffer, w_str1, wcslen(w_str1) + 2); // expected-warning {{Memory copy function accesses out-of-bound array element}} +} + +void test_strnlen(void) { + const char str1[] = "0123456789"; + const char str2[] = ""; + clang_analyzer_eval(strnlen(str1, 1) == 1); // expected-warning {{TRUE}} + clang_analyzer_eval(strnlen(str1, 100) == 10); // expected-warning {{TRUE}} + clang_analyzer_eval(strnlen(str2, 10) == 0); // expected-warning {{TRUE}} +} + +void w_test_wcsnlen(void) { + const wchar_t str1[] = L"0123456789"; + const wchar_t str2[] = L""; + clang_analyzer_eval(wcsnlen(str1, 1) == 1); // expected-warning {{TRUE}} + clang_analyzer_eval(wcsnlen(str1, 100) == 10); // expected-warning {{TRUE}} + clang_analyzer_eval(wcsnlen(str2, 10) == 0); // expected-warning {{TRUE}} +} + +void test_strlen_with_zero(void) { + char buffer[32]; + const char str1[] = "Hello\0world"; + clang_analyzer_eval(strlen(str1) == 11); // expected-warning {{TRUE}} +} + +void test_dst_overflow(void) { + char buffer[] = "Helloworld"; + memcpy(buffer, str, sizeof(str)); // expected-warning {{Memory copy function overflows the destination buffer [alpha.unix.cstring.OutOfBounds]}} +} + +void w_test_dst_overflow(void) { + wchar_t buffer[] = L"Helloworld"; + wmemcpy(buffer, w_str, sizeof(w_str) / sizeof(w_str[0])); // expected-warning {{Memory copy function overflows the destination buffer}} +} + +void test_dst_offset(void) { + char buffer[] = "Hello world"; + memcpy(buffer + 1, str, sizeof(str)); // expected-warning {{Memory copy function overflows the destination buffer}} +} + +void w_test_dst_offset(void) { + wchar_t buffer[] = L"Hello world"; + wmemcpy(buffer + 1, w_str, sizeof(w_str) / sizeof(w_str[0])); // expected-warning {{Memory copy function overflows the destination buffer}} +} + +int test_strlen_null() { + return strlen(0); // expected-warning {{Null pointer passed as 1st argument to string length function [unix.cstring.NullArg]}} +} + +int test_wcslen_null() { + return wcslen(0); // expected-warning {{Null pointer passed as 1st argument to string length function}} +} + +int test_strnlen_null() { + return strnlen(0, 10); // expected-warning {{Null pointer passed as 1st argument to string length function}} +} + +int test_wcsnlen_null() { + return wcsnlen(0, 10); // expected-warning {{Null pointer passed as 1st argument to string length function}} +} + +void test_overlap() { + int a[4] = {0}; + memcpy(a + 2, a + 1, sizeof(int)); + memcpy(a + 2, a + 1, sizeof(int) * 2); // expected-warning {{Arguments must not be overlapping buffers [alpha.unix.cstring.BufferOverlap]}} +} + +void w_test_overlap() { + wchar_t a[4] = {0}; + memcpy(a + 2, a + 1, sizeof(wchar_t)); + memcpy(a + 2, a + 1, sizeof(wchar_t) * 2); // expected-warning {{Arguments must not be overlapping buffers}} +} + +void test_not_null_terminated() { + int y = strlen((char *)&test_not_null_terminated); // expected-warning {{Argument to string length function is the address of the function 'test_not_null_terminated', which is not a null-terminated string [alpha.unix.cstring.NotNullTerminated]}} +} + +void w_test_not_null_terminated() { + int y = wcslen((wchar_t *)&w_test_not_null_terminated); // expected-warning {{Argument to string length function is the address of the function 'w_test_not_null_terminated', which is not a null-terminated string}} +}