shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

[analyzer] CStringChecker should check the first byte of the destination of strcpy, strncpy

Browse Source 
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
This commit is contained in:
Balazs Benics 2023-09-11 14:19:33 +02:00
parent 4b9259b947
commit c3a87ddad6
2 changed files with 56 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
View File

@ -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;

46
clang/test/Analysis/string.c
View File

@ -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