Skip to content

Commit ba535f8

Browse files
balazskebalazske
committed
[clang][analyzer] Improve checker 'unix.cstring.NotNullTerminated'
Check for non-presence of terminating zero in simple cases.
1 parent db2eb4d commit ba535f8

File tree

4 files changed

+263
-11
lines changed

4 files changed

+263
-11
lines changed

‎clang/docs/analyzer/checkers.rst‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2105,10 +2105,11 @@ unix.cstring.NotNullTerminated (C)
21052105
Check for arguments which are not null-terminated strings;
21062106
applies to the ``strlen``, ``strcpy``, ``strcat``, ``strcmp`` family of functions.
21072107
2108-
Only very fundamental cases are detected where the passed memory block is
2109-
absolutely different from a null-terminated string. This checker does not
2110-
find if a memory buffer is passed where the terminating zero character
2111-
is missing.
2108+
The checker can detect if the passed memory block is not a string object at all,
2109+
like address of a label or function. Additionally it can detect simple cases
2110+
when the terminating zero is missing, for example if the data was initialized
2111+
as array without terminating zero, or the terminating zero was overwritten by
2112+
an assignment (with a value that is provably not zero).
21122113
21132114
.. code-block:: c
21142115
@@ -2121,6 +2122,17 @@ is missing.
21212122
int l = strlen((char *)&&label); // warn
21222123
}
21232124
2125+
int test3() {
2126+
char buf[4] = {1, 2, 3, 4};
2127+
return strlen(buf); // warn
2128+
}
2129+
2130+
int test4() {
2131+
char buf[] = "abcd";
2132+
buf[4] = 'e';
2133+
return strlen(buf); // warn
2134+
}
2135+
21242136
.. _unix-cstring-NullArg:
21252137
21262138
unix.cstring.NullArg (C)

‎clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp‎

Lines changed: 87 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,9 @@ static QualType getCharPtrType(ASTContext &Ctx, CharKind CK) {
7878
: Ctx.WideCharTy);
7979
}
8080

81-
class CStringChecker : public Checker< eval::Call,
82-
check::PreStmt<DeclStmt>,
83-
check::LiveSymbols,
84-
check::DeadSymbols,
85-
check::RegionChanges
86-
> {
81+
class CStringChecker
82+
: public Checker<eval::Call, check::PreStmt<DeclStmt>, check::LiveSymbols,
83+
check::DeadSymbols, check::RegionChanges> {
8784
mutable std::unique_ptr<BugType> BT_Null, BT_Bounds, BT_Overlap,
8885
BT_NotCString, BT_AdditionOverflow, BT_UninitRead;
8986

@@ -324,11 +321,14 @@ class CStringChecker : public Checker< eval::Call,
324321
SizeArgExpr Size, AnyArgExpr First,
325322
AnyArgExpr Second,
326323
CharKind CK = CharKind::Regular) const;
324+
// Check for presence of terminating zero in a string argument.
325+
ProgramStateRef checkNullTerminated(CheckerContext &C, ProgramStateRef State,
326+
AnyArgExpr Arg, SVal ArgVal) const;
327+
327328
void emitOverlapBug(CheckerContext &C,
328329
ProgramStateRef state,
329330
const Stmt *First,
330331
const Stmt *Second) const;
331-
332332
void emitNullArgBug(CheckerContext &C, ProgramStateRef State, const Stmt *S,
333333
StringRef WarningMsg) const;
334334
void emitOutOfBoundsBug(CheckerContext &C, ProgramStateRef State,
@@ -959,6 +959,66 @@ ProgramStateRef CStringChecker::checkAdditionOverflow(CheckerContext &C,
959959
return state;
960960
}
961961

962+
ProgramStateRef CStringChecker::checkNullTerminated(CheckerContext &C,
963+
ProgramStateRef State,
964+
AnyArgExpr Arg,
965+
SVal ArgVal) const {
966+
if (!State)
967+
return nullptr;
968+
969+
if (!Filter.CheckCStringNotNullTerm)
970+
return State;
971+
972+
SValBuilder &SVB = C.getSValBuilder();
973+
974+
auto TryGetTypedValueR = [](const MemRegion *R) -> const TypedValueRegion * {
975+
if (!R)
976+
return nullptr;
977+
return R->StripCasts()->getAs<TypedValueRegion>();
978+
};
979+
980+
const TypedValueRegion *StrReg = TryGetTypedValueR(ArgVal.getAsRegion());
981+
if (!StrReg)
982+
return State;
983+
int Offset = 0;
984+
if (const auto *ElemReg = StrReg->getAs<ElementRegion>()) {
985+
RegionRawOffset ROffset = ElemReg->getAsArrayOffset();
986+
StrReg = TryGetTypedValueR(ROffset.getRegion());
987+
if (!StrReg)
988+
return State;
989+
Offset = ROffset.getOffset().getQuantity();
990+
}
991+
992+
DefinedOrUnknownSVal Extent = getDynamicExtent(State, StrReg, SVB);
993+
if (Extent.isUnknown())
994+
return State;
995+
const llvm::APSInt *KnownExtent = SVB.getKnownValue(State, Extent);
996+
if (!KnownExtent)
997+
return State;
998+
MemRegionManager &RegionM = SVB.getRegionManager();
999+
int RegionExtent = KnownExtent->getExtValue();
1000+
if (Offset >= RegionExtent)
1001+
return State;
1002+
for (int I = Offset; I < RegionExtent; ++I) {
1003+
const ElementRegion *ElemR = RegionM.getElementRegion(C.getASTContext().CharTy, SVB.makeArrayIndex(I), StrReg, C.getASTContext());
1004+
SVal ElemVal = State->getSValAsScalarOrLoc(ElemR);
1005+
if (!State->isNonNull(ElemVal).isConstrainedTrue())
1006+
// We have here a lower bound for the string length.
1007+
// Try to update the CStringLength value?
1008+
return State;
1009+
}
1010+
1011+
SmallString<80> Buf;
1012+
llvm::raw_svector_ostream OS(Buf);
1013+
assert(CurrentFunctionDescription);
1014+
OS << "Terminating zero missing from string passed as "
1015+
<< (Arg.ArgumentIndex + 1) << llvm::getOrdinalSuffix(Arg.ArgumentIndex + 1)
1016+
<< " argument to " << CurrentFunctionDescription;
1017+
1018+
emitNotCStringBug(C, State, Arg.Expression, OS.str());
1019+
return nullptr;
1020+
}
1021+
9621022
ProgramStateRef CStringChecker::setCStringLength(ProgramStateRef state,
9631023
const MemRegion *MR,
9641024
SVal strLength) {
@@ -1718,6 +1778,9 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C,
17181778
SVal ArgVal = state->getSVal(Arg.Expression, LCtx);
17191779
state = checkNonNull(C, state, Arg, ArgVal);
17201780

1781+
if (!IsStrnlen)
1782+
state = checkNullTerminated(C, state, Arg, ArgVal);
1783+
17211784
if (!state)
17221785
return;
17231786

@@ -1882,13 +1945,19 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call,
18821945
DestinationArgExpr Dst = {{Call.getArgExpr(0), 0}};
18831946
SVal DstVal = state->getSVal(Dst.Expression, LCtx);
18841947
state = checkNonNull(C, state, Dst, DstVal);
1948+
// Look for terminating zero.
1949+
if (!IsBounded || appendK != ConcatFnKind::none)
1950+
state = checkNullTerminated(C, state, Dst, DstVal);
18851951
if (!state)
18861952
return;
18871953

18881954
// Check that the source is non-null.
18891955
SourceArgExpr srcExpr = {{Call.getArgExpr(1), 1}};
18901956
SVal srcVal = state->getSVal(srcExpr.Expression, LCtx);
18911957
state = checkNonNull(C, state, srcExpr, srcVal);
1958+
// Look for terminating zero.
1959+
if (!IsBounded)
1960+
state = checkNullTerminated(C, state, srcExpr, srcVal);
18921961
if (!state)
18931962
return;
18941963

@@ -2340,13 +2409,19 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallEvent &Call,
23402409
AnyArgExpr Left = {Call.getArgExpr(0), 0};
23412410
SVal LeftVal = state->getSVal(Left.Expression, LCtx);
23422411
state = checkNonNull(C, state, Left, LeftVal);
2412+
// Look for terminating zero.
2413+
if (!IsBounded)
2414+
state = checkNullTerminated(C, state, Left, LeftVal);
23432415
if (!state)
23442416
return;
23452417

23462418
// Check that the second string is non-null.
23472419
AnyArgExpr Right = {Call.getArgExpr(1), 1};
23482420
SVal RightVal = state->getSVal(Right.Expression, LCtx);
23492421
state = checkNonNull(C, state, Right, RightVal);
2422+
// Look for terminating zero.
2423+
if (!IsBounded)
2424+
state = checkNullTerminated(C, state, Right, RightVal);
23502425
if (!state)
23512426
return;
23522427

@@ -2478,13 +2553,17 @@ void CStringChecker::evalStrsep(CheckerContext &C,
24782553
// a null string).
24792554
SVal SearchStrVal = State->getSVal(SearchStrPtr.Expression, LCtx);
24802555
State = checkNonNull(C, State, SearchStrPtr, SearchStrVal);
2556+
// Look for terminating zero.
2557+
State = checkNullTerminated(C, State, SearchStrPtr, SearchStrVal);
24812558
if (!State)
24822559
return;
24832560

24842561
// Check that the delimiter string is non-null.
24852562
AnyArgExpr DelimStr = {Call.getArgExpr(1), 1};
24862563
SVal DelimStrVal = State->getSVal(DelimStr.Expression, LCtx);
24872564
State = checkNonNull(C, State, DelimStr, DelimStrVal);
2565+
// Look for terminating zero.
2566+
State = checkNullTerminated(C, State, DelimStr, DelimStrVal);
24882567
if (!State)
24892568
return;
24902569

@@ -2675,6 +2754,7 @@ void CStringChecker::evalSprintfCommon(CheckerContext &C, const CallEvent &Call,
26752754
// This is an invalid call, let's just ignore it.
26762755
return;
26772756
}
2757+
// FIXME: Why not check for null pointer (and null-terminated string)?
26782758

26792759
const auto AllArguments =
26802760
llvm::make_range(CE->getArgs(), CE->getArgs() + CE->getNumArgs());

‎clang/test/Analysis/Inputs/system-header-simulator.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ size_t strlen(const char *);
8080

8181
char *strcpy(char *restrict, const char *restrict);
8282
char *strncpy(char *restrict dst, const char *restrict src, size_t n);
83+
char *strcat(char *restrict, const char *restrict);
84+
char *strncat(char *restrict, const char *restrict, size_t);
8385
char *strsep(char **restrict stringp, const char *restrict delim);
8486
void *memcpy(void *restrict dst, const void *restrict src, size_t n);
8587
void *memset(void *s, int c, size_t n);

‎clang/test/Analysis/cstring-notnullterm.c‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.NotNullTerminated -verify %s
2+
3+
#include "Inputs/system-header-simulator.h"
4+
5+
extern void *malloc(size_t);
6+
7+
size_t test_addr_fn() {
8+
return strlen((char *)&malloc); // expected-warning{{Argument to string length function is the address of the function 'malloc', which is not a null-terminated string}}
9+
}
10+
11+
size_t test_addr_label() {
12+
lab:
13+
return strlen((char *)&&lab); // expected-warning{{Argument to string length function is the address of the label 'lab', which is not a null-terminated string}}
14+
}
15+
16+
size_t test_init_compound(int i) {
17+
char src1[6] = {1,2,3,4,5,6};
18+
char src2[6] = {1,2,3,0,5,6};
19+
switch (i) {
20+
case 1:
21+
return strlen(src1); // expected-warning{{Terminating zero missing from string passed as 1st argument to string length function}}
22+
case 2:
23+
return strlen(src1 + 1); // expected-warning{{Terminating zero missing from string}}
24+
case 3:
25+
return strlen(src2);
26+
case 4:
27+
return strlen(src2 + 4); // expected-warning{{Terminating zero missing from string}}
28+
case 5:
29+
return strlen(src2 + 3);
30+
}
31+
src1[5] = 0;
32+
return strlen(src1);
33+
}
34+
35+
size_t test_init_literal(int i) {
36+
char src1[] = "abcdef";
37+
int l = strlen(src1);
38+
src1[6] = '.';
39+
src1[3] = 0;
40+
switch (i) {
41+
case 1:
42+
return strlen(src1);
43+
case 2:
44+
return strlen(src1 + 4); // expected-warning{{Terminating zero missing from string}}
45+
}
46+
return l;
47+
}
48+
49+
size_t test_init_assign(int i, char a) {
50+
char src[6];
51+
src[1] = '1';
52+
src[2] = '2';
53+
src[4] = '4';
54+
src[5] = '5';
55+
56+
switch (i) {
57+
case 0:
58+
return strlen(src);
59+
case 1:
60+
return strlen(src + 1);
61+
case 2:
62+
return strlen(src + 2);
63+
case 3:
64+
return strlen(src + 3);
65+
case 4:
66+
return strlen(src + 4); // expected-warning{{Terminating zero missing from string}}
67+
}
68+
src[5] = a;
69+
size_t l = strlen(src + 4);
70+
src[5] = 0;
71+
l += strlen(src + 4);
72+
src[5] = '5';
73+
return l + strlen(src + 4); // expected-warning{{Terminating zero missing from string}}
74+
}
75+
76+
size_t test_assign1() {
77+
char str1[5] = {'0','1','2','3','4'};
78+
char str2[5];
79+
str2[0] = str1[0];
80+
str2[1] = str1[1];
81+
str2[4] = str1[4];
82+
size_t l = strlen(str2);
83+
return l + strlen(str2 + 4); // expected-warning{{Terminating zero missing from string}}
84+
}
85+
86+
size_t test_assign2() {
87+
char str1[5] = {1,2,3,4,5};
88+
char str2[5];
89+
str2[0] = str1[0];
90+
str2[4] = str2[0];
91+
return strlen(str2 + 4); // expected-warning{{Terminating zero missing from string}}
92+
}
93+
94+
void test_ignore(char *dst) {
95+
char str1[5] = {1,2,3,4,5};
96+
strncpy(dst, str1, 5);
97+
strcpy(dst, str1); // expected-warning{{Terminating zero missing from string}}
98+
}
99+
100+
size_t test_malloc() {
101+
char *buf = (char *)malloc(4);
102+
if (!buf)
103+
return 0;
104+
buf[3] = 'a';
105+
return strlen(buf);
106+
}
107+
108+
extern void f_ext(char *);
109+
char *g_buf = 0;
110+
111+
size_t test_escape1() {
112+
char buf[4] = {1,2,3,4};
113+
f_ext(buf);
114+
return strlen(buf);
115+
}
116+
117+
size_t test_escape2(char *x) {
118+
char buf[4] = {1,2,3,4};
119+
g_buf = buf;
120+
f_ext(x);
121+
return strlen(buf);
122+
}
123+
124+
size_t test_escape3() {
125+
char buf[4] = {1,2,3,4};
126+
f_ext(buf + 3);
127+
return strlen(buf);
128+
}
129+
130+
void test_str_fn(int i, char *dst) {
131+
char buf[] = {1, 2, 3};
132+
switch (i) {
133+
case 1:
134+
strcpy(buf, "aa"); // expected-warning{{Terminating zero missing from string}}
135+
break;
136+
case 2:
137+
strcpy(dst, buf); // expected-warning{{Terminating zero missing from string}}
138+
break;
139+
case 3:
140+
strncpy(buf, "aa", 3);
141+
break;
142+
case 4:
143+
strncpy(dst, buf, 3);
144+
break;
145+
case 5:
146+
strcat(buf, "aa"); // expected-warning{{Terminating zero missing from string}}
147+
break;
148+
case 6:
149+
strcat(dst, buf); // expected-warning{{Terminating zero missing from string}}
150+
break;
151+
case 7:
152+
strncat(buf, "aa", 3); // expected-warning{{Terminating zero missing from string}}
153+
break;
154+
case 8:
155+
strncat(dst, buf, 3);
156+
break;
157+
}
158+
}

0 commit comments

Comments
 (0)