Skip to content

Commit de2ec22

Browse files
vbvictorvbvictor
authored
[analyzer] Fix crash when modelling 'getline' function in checkers (#145229)
Fixes #144884
1 parent 9e33cb2 commit de2ec22

File tree

4 files changed

+177
-25
lines changed

4 files changed

+177
-25
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,6 +1071,9 @@ impact the linker behaviour like the other `-static-*` flags.
10711071
Crash and bug fixes
10721072
^^^^^^^^^^^^^^^^^^^
10731073

1074+
- Fixed a crash in ``UnixAPIMisuseChecker`` and ``MallocChecker`` when analyzing
1075+
code with non-standard ``getline`` or ``getdelim`` function signatures. (#GH144884)
1076+
10741077
Improvements
10751078
^^^^^^^^^^^^
10761079

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

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,8 @@ class MallocChecker
430430
CHECK_FN(checkGMemdup)
431431
CHECK_FN(checkGMallocN)
432432
CHECK_FN(checkGMallocN0)
433-
CHECK_FN(preGetdelim)
434-
CHECK_FN(checkGetdelim)
433+
CHECK_FN(preGetDelimOrGetLine)
434+
CHECK_FN(checkGetDelimOrGetLine)
435435
CHECK_FN(checkReallocN)
436436
CHECK_FN(checkOwnershipAttr)
437437

@@ -445,15 +445,16 @@ class MallocChecker
445445
const CallDescriptionMap<CheckFn> PreFnMap{
446446
// NOTE: the following CallDescription also matches the C++ standard
447447
// library function std::getline(); the callback will filter it out.
448-
{{CDM::CLibrary, {"getline"}, 3}, &MallocChecker::preGetdelim},
449-
{{CDM::CLibrary, {"getdelim"}, 4}, &MallocChecker::preGetdelim},
448+
{{CDM::CLibrary, {"getline"}, 3}, &MallocChecker::preGetDelimOrGetLine},
449+
{{CDM::CLibrary, {"getdelim"}, 4}, &MallocChecker::preGetDelimOrGetLine},
450450
};
451451

452452
const CallDescriptionMap<CheckFn> PostFnMap{
453453
// NOTE: the following CallDescription also matches the C++ standard
454454
// library function std::getline(); the callback will filter it out.
455-
{{CDM::CLibrary, {"getline"}, 3}, &MallocChecker::checkGetdelim},
456-
{{CDM::CLibrary, {"getdelim"}, 4}, &MallocChecker::checkGetdelim},
455+
{{CDM::CLibrary, {"getline"}, 3}, &MallocChecker::checkGetDelimOrGetLine},
456+
{{CDM::CLibrary, {"getdelim"}, 4},
457+
&MallocChecker::checkGetDelimOrGetLine},
457458
};
458459

459460
const CallDescriptionMap<CheckFn> FreeingMemFnMap{
@@ -1482,8 +1483,9 @@ static bool isFromStdNamespace(const CallEvent &Call) {
14821483
return FD->isInStdNamespace();
14831484
}
14841485

1485-
void MallocChecker::preGetdelim(ProgramStateRef State, const CallEvent &Call,
1486-
CheckerContext &C) const {
1486+
void MallocChecker::preGetDelimOrGetLine(ProgramStateRef State,
1487+
const CallEvent &Call,
1488+
CheckerContext &C) const {
14871489
// Discard calls to the C++ standard library function std::getline(), which
14881490
// is completely unrelated to the POSIX getline() that we're checking.
14891491
if (isFromStdNamespace(Call))
@@ -1505,8 +1507,9 @@ void MallocChecker::preGetdelim(ProgramStateRef State, const CallEvent &Call,
15051507
C.addTransition(State);
15061508
}
15071509

1508-
void MallocChecker::checkGetdelim(ProgramStateRef State, const CallEvent &Call,
1509-
CheckerContext &C) const {
1510+
void MallocChecker::checkGetDelimOrGetLine(ProgramStateRef State,
1511+
const CallEvent &Call,
1512+
CheckerContext &C) const {
15101513
// Discard calls to the C++ standard library function std::getline(), which
15111514
// is completely unrelated to the POSIX getline() that we're checking.
15121515
if (isFromStdNamespace(Call))
@@ -1518,14 +1521,19 @@ void MallocChecker::checkGetdelim(ProgramStateRef State, const CallEvent &Call,
15181521
if (!CE)
15191522
return;
15201523

1521-
const auto LinePtr =
1522-
getPointeeVal(Call.getArgSVal(0), State)->getAs<DefinedSVal>();
1523-
const auto Size =
1524-
getPointeeVal(Call.getArgSVal(1), State)->getAs<DefinedSVal>();
1525-
if (!LinePtr || !Size || !LinePtr->getAsRegion())
1524+
const auto LinePtrOpt = getPointeeVal(Call.getArgSVal(0), State);
1525+
const auto SizeOpt = getPointeeVal(Call.getArgSVal(1), State);
1526+
if (!LinePtrOpt || !SizeOpt || LinePtrOpt->isUnknownOrUndef() ||
1527+
SizeOpt->isUnknownOrUndef())
1528+
return;
1529+
1530+
const auto LinePtr = LinePtrOpt->getAs<DefinedSVal>();
1531+
const auto Size = SizeOpt->getAs<DefinedSVal>();
1532+
const MemRegion *LinePtrReg = LinePtr->getAsRegion();
1533+
if (!LinePtrReg)
15261534
return;
15271535

1528-
State = setDynamicExtent(State, LinePtr->getAsRegion(), *Size);
1536+
State = setDynamicExtent(State, LinePtrReg, *Size);
15291537
C.addTransition(MallocUpdateRefState(C, CE, State,
15301538
AllocationFamily(AF_Malloc), *LinePtr));
15311539
}

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

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ class UnixAPIMisuseChecker : public Checker<check::PreCall> {
8383

8484
void CheckOpen(CheckerContext &C, const CallEvent &Call) const;
8585
void CheckOpenAt(CheckerContext &C, const CallEvent &Call) const;
86-
void CheckGetDelim(CheckerContext &C, const CallEvent &Call) const;
86+
void CheckGetDelimOrGetline(CheckerContext &C, const CallEvent &Call) const;
8787
void CheckPthreadOnce(CheckerContext &C, const CallEvent &Call) const;
8888

8989
void CheckOpenVariant(CheckerContext &C, const CallEvent &Call,
@@ -128,7 +128,7 @@ ProgramStateRef UnixAPIMisuseChecker::EnsurePtrNotNull(
128128
const StringRef PtrDescr,
129129
std::optional<std::reference_wrapper<const BugType>> BT) const {
130130
const auto Ptr = PtrVal.getAs<DefinedSVal>();
131-
if (!Ptr)
131+
if (!Ptr || !PtrExpr->getType()->isPointerType())
132132
return State;
133133

134134
const auto [PtrNotNull, PtrNull] = State->assume(*Ptr);
@@ -177,7 +177,7 @@ void UnixAPIMisuseChecker::checkPreCall(const CallEvent &Call,
177177
CheckPthreadOnce(C, Call);
178178

179179
else if (is_contained({"getdelim", "getline"}, FName))
180-
CheckGetDelim(C, Call);
180+
CheckGetDelimOrGetline(C, Call);
181181
}
182182
void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C,
183183
ProgramStateRef State,
@@ -332,8 +332,12 @@ ProgramStateRef UnixAPIMisuseChecker::EnsureGetdelimBufferAndSizeCorrect(
332332

333333
// We have a pointer to a pointer to the buffer, and a pointer to the size.
334334
// We want what they point at.
335-
auto LinePtrSVal = getPointeeVal(LinePtrPtrSVal, State)->getAs<DefinedSVal>();
336-
auto NSVal = getPointeeVal(SizePtrSVal, State);
335+
const auto LinePtrValOpt = getPointeeVal(LinePtrPtrSVal, State);
336+
if (!LinePtrValOpt)
337+
return nullptr;
338+
339+
const auto LinePtrSVal = LinePtrValOpt->getAs<DefinedSVal>();
340+
const auto NSVal = getPointeeVal(SizePtrSVal, State);
337341
if (!LinePtrSVal || !NSVal || NSVal->isUnknown())
338342
return nullptr;
339343

@@ -350,9 +354,16 @@ ProgramStateRef UnixAPIMisuseChecker::EnsureGetdelimBufferAndSizeCorrect(
350354
// If it is defined, and known, its size must be less than or equal to
351355
// the buffer size.
352356
auto NDefSVal = NSVal->getAs<DefinedSVal>();
357+
if (!NDefSVal)
358+
return LinePtrNotNull;
359+
353360
auto &SVB = C.getSValBuilder();
354-
auto LineBufSize =
355-
getDynamicExtent(LinePtrNotNull, LinePtrSVal->getAsRegion(), SVB);
361+
362+
const MemRegion *LinePtrRegion = LinePtrSVal->getAsRegion();
363+
if (!LinePtrRegion)
364+
return LinePtrNotNull;
365+
366+
auto LineBufSize = getDynamicExtent(LinePtrNotNull, LinePtrRegion, SVB);
356367
auto LineBufSizeGtN = SVB.evalBinOp(LinePtrNotNull, BO_GE, LineBufSize,
357368
*NDefSVal, SVB.getConditionType())
358369
.getAs<DefinedOrUnknownSVal>();
@@ -367,8 +378,11 @@ ProgramStateRef UnixAPIMisuseChecker::EnsureGetdelimBufferAndSizeCorrect(
367378
return State;
368379
}
369380

370-
void UnixAPIMisuseChecker::CheckGetDelim(CheckerContext &C,
371-
const CallEvent &Call) const {
381+
void UnixAPIMisuseChecker::CheckGetDelimOrGetline(CheckerContext &C,
382+
const CallEvent &Call) const {
383+
if (Call.getNumArgs() < 2)
384+
return;
385+
372386
ProgramStateRef State = C.getState();
373387

374388
// The parameter `n` must not be NULL.

‎clang/test/Analysis/getline-unixapi-invalid-signatures.c

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_CORRECT
2+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_1
3+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_2
4+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_3
5+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_4
6+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_5
7+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_GH144884
8+
9+
// emulator of "system-header-simulator.h" because of redefinition of 'getline' function
10+
typedef struct _FILE FILE;
11+
typedef __typeof(sizeof(int)) size_t;
12+
typedef long ssize_t;
13+
#define NULL 0
14+
15+
int fclose(FILE *fp);
16+
FILE *tmpfile(void);
17+
18+
#ifdef TEST_CORRECT
19+
ssize_t getline(char **lineptr, size_t *n, FILE *stream);
20+
ssize_t getdelim(char **lineptr, size_t *n, int delimiter, FILE *stream);
21+
22+
void test_correct() {
23+
FILE *F1 = tmpfile();
24+
if (!F1)
25+
return;
26+
char *buffer = NULL;
27+
getline(&buffer, NULL, F1); // expected-warning {{Size pointer might be NULL}}
28+
fclose(F1);
29+
}
30+
31+
void test_delim_correct() {
32+
FILE *F1 = tmpfile();
33+
if (!F1)
34+
return;
35+
char *buffer = NULL;
36+
getdelim(&buffer, NULL, ',', F1); // expected-warning {{Size pointer might be NULL}}
37+
fclose(F1);
38+
}
39+
#endif
40+
41+
#ifdef TEST_GETLINE_1
42+
// expected-no-diagnostics
43+
ssize_t getline(int lineptr);
44+
45+
void test() {
46+
FILE *F1 = tmpfile();
47+
if (!F1)
48+
return;
49+
int buffer = 0;
50+
getline(buffer);
51+
fclose(F1);
52+
}
53+
#endif
54+
55+
#ifdef TEST_GETLINE_2
56+
ssize_t getline(char **lineptr, size_t *n);
57+
58+
void test() {
59+
FILE *F1 = tmpfile();
60+
if (!F1)
61+
return;
62+
char *buffer = NULL;
63+
getline(&buffer, NULL); // expected-warning {{Size pointer might be NULL}}
64+
fclose(F1);
65+
}
66+
#endif
67+
68+
#ifdef TEST_GETLINE_3
69+
// expected-no-diagnostics
70+
ssize_t getline(char **lineptr, size_t n, FILE *stream);
71+
72+
void test() {
73+
FILE *F1 = tmpfile();
74+
if (!F1)
75+
return;
76+
char *buffer = NULL;
77+
getline(&buffer, 0, F1);
78+
fclose(F1);
79+
}
80+
#endif
81+
82+
#ifdef TEST_GETLINE_4
83+
ssize_t getline(char **lineptr, size_t *n, int stream);
84+
ssize_t getdelim(char **lineptr, size_t *n, int delimiter, int stream);
85+
86+
void test() {
87+
FILE *F1 = tmpfile();
88+
if (!F1)
89+
return;
90+
char *buffer = NULL;
91+
getline(&buffer, NULL, 1); // expected-warning {{Size pointer might be NULL}}
92+
fclose(F1);
93+
}
94+
95+
void test_delim() {
96+
FILE *F1 = tmpfile();
97+
if (!F1)
98+
return;
99+
char *buffer = NULL;
100+
getdelim(&buffer, NULL, ',', 1); // expected-warning {{Size pointer might be NULL}}
101+
fclose(F1);
102+
}
103+
#endif
104+
105+
#ifdef TEST_GETLINE_5
106+
ssize_t getdelim(char **lineptr, size_t *n, const char* delimiter, FILE *stream);
107+
108+
void test_delim() {
109+
FILE *F1 = tmpfile();
110+
if (!F1)
111+
return;
112+
char *buffer = NULL;
113+
getdelim(&buffer, NULL, ",", F1); // expected-warning {{Size pointer might be NULL}}
114+
fclose(F1);
115+
}
116+
#endif
117+
118+
#ifdef TEST_GETLINE_GH144884
119+
// expected-no-diagnostics
120+
struct AW_string {};
121+
void getline(int *, struct AW_string);
122+
void top() {
123+
struct AW_string line;
124+
int getline_file_info;
125+
getline(&getline_file_info, line);
126+
}
127+
#endif

0 commit comments

Comments
 (0)