Author: Balazs Benics Date: 2023-09-11T14:19:33+02:00 New Revision: c3a87ddad62a6cc01acaccc76592bc6730c8ac3c
URL: https://github.com/llvm/llvm-project/commit/c3a87ddad62a6cc01acaccc76592bc6730c8ac3c DIFF: https://github.com/llvm/llvm-project/commit/c3a87ddad62a6cc01acaccc76592bc6730c8ac3c.diff LOG: [analyzer] CStringChecker should check the first byte of the destination of strcpy, strncpy By not checking if the first byte of the destination of strcpy and strncpy is writable, we missed some reports in the Juliet benchmark. (Juliet CWE-124 Buffer Underwrite: strcpy, strncpy) https://discourse.llvm.org/t/patches-inspired-by-the-juliet-benchmark/73106 Differential Revision: https://reviews.llvm.org/D159108 Added: Modified: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp clang/test/Analysis/string.c Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 7d3aaac1ec0a842..66ed78e4b17a72c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -2009,6 +2009,11 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, SVal maxLastElement = svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal, *maxLastNL, ptrTy); + // Check if the first byte of the destination is writable. + state = CheckLocation(C, state, Dst, DstVal, AccessKind::write); + if (!state) + return; + // Check if the last byte of the destination is writable. state = CheckLocation(C, state, Dst, maxLastElement, AccessKind::write); if (!state) return; @@ -2021,6 +2026,11 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, // ...and we haven't checked the bound, we'll check the actual copy. if (!boundWarning) { + // Check if the first byte of the destination is writable. + state = CheckLocation(C, state, Dst, DstVal, AccessKind::write); + if (!state) + return; + // Check if the last byte of the destination is writable. state = CheckLocation(C, state, Dst, lastElement, AccessKind::write); if (!state) return; diff --git a/clang/test/Analysis/string.c b/clang/test/Analysis/string.c index 6bdf59c56f63b8a..476f44d7830d33b 100644 --- a/clang/test/Analysis/string.c +++ b/clang/test/Analysis/string.c @@ -1667,3 +1667,49 @@ void strcpy_no_overflow_2(char *y) { strcpy(x, "12\0"); } #endif + +#ifndef SUPPRESS_OUT_OF_BOUND +void testStrcpyDestinationWritableFirstByte(void) { + char dst[10]; + char *p = dst - 8; + strcpy(p, "src"); // expected-warning {{String copy function overflows the destination buffer}} +} + +void CWE124_Buffer_Underwrite__malloc_char_cpy() { + char * dataBuffer = (char *)malloc(100*sizeof(char)); + if (dataBuffer == NULL) return; + memset(dataBuffer, 'A', 100-1); + dataBuffer[100-1] = '\0'; + char * data = dataBuffer - 8; + char source[100]; + memset(source, 'C', 100-1); // fill with 'C's + source[100-1] = '\0'; // null terminate + strcpy(data, source); // expected-warning {{String copy function overflows the destination buffer}} + free(dataBuffer); +} +#endif + +#ifndef SUPPRESS_OUT_OF_BOUND +void testStrncpyDestinationWritableFirstByte(void) { + char source[100]; + use_string(source); // escape + char buf[100]; + char *p = buf - 8; + strncpy(p, source, 100-1); // expected-warning {{String copy function overflows the destination buffer}} +} + +void CWE124_Buffer_Underwrite__malloc_char_ncpy() { + char * dataBuffer = (char *)malloc(100*sizeof(char)); + if (dataBuffer == 0) return; + memset(dataBuffer, 'A', 100-1); + dataBuffer[100-1] = '\0'; + char *data = dataBuffer - 8; + + char source[100]; + memset(source, 'C', 100-1); // fill with 'C's + source[100-1] = '\0'; // null terminate + strncpy(data, source, 100-1); // expected-warning {{String copy function overflows the destination buffer}} + data[100-1] = '\0'; // null terminate + free(dataBuffer); +} +#endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
