Skip to content

Commit 5cecea4

Browse files
authored
Merge pull request github#6603 from geoffw0/impropnulltests
C++: Add test cases for cpp/improper-null-termination.
2 parents b7206c1 + 4e60fd5 commit 5cecea4

File tree

4 files changed

+243
-5
lines changed

4 files changed

+243
-5
lines changed

cpp/ql/test/query-tests/Likely Bugs/Memory Management/ImproperNullTermination/ImproperNullTermination.expected

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,15 @@
1111
| test.cpp:130:14:130:19 | buffer | Variable $@ may not be null terminated. | test.cpp:127:7:127:12 | buffer | buffer |
1212
| test.cpp:139:10:139:15 | buffer | Variable $@ may not be null terminated. | test.cpp:136:8:136:13 | buffer | buffer |
1313
| test.cpp:147:14:147:19 | buffer | Variable $@ may not be null terminated. | test.cpp:143:8:143:13 | buffer | buffer |
14-
| test.cpp:170:10:170:15 | buffer | Variable $@ may not be null terminated. | test.cpp:166:8:166:13 | buffer | buffer |
14+
| test.cpp:182:10:182:15 | buffer | Variable $@ may not be null terminated. | test.cpp:178:8:178:13 | buffer | buffer |
15+
| test.cpp:234:10:234:15 | buffer | Variable $@ may not be null terminated. | test.cpp:232:8:232:13 | buffer | buffer |
16+
| test.cpp:262:10:262:15 | buffer | Variable $@ may not be null terminated. | test.cpp:259:8:259:13 | buffer | buffer |
17+
| test.cpp:283:10:283:15 | buffer | Variable $@ may not be null terminated. | test.cpp:280:8:280:13 | buffer | buffer |
18+
| test.cpp:300:10:300:16 | buffer2 | Variable $@ may not be null terminated. | test.cpp:295:8:295:14 | buffer2 | buffer2 |
19+
| test.cpp:312:10:312:15 | buffer | Variable $@ may not be null terminated. | test.cpp:308:8:308:13 | buffer | buffer |
20+
| test.cpp:327:18:327:23 | buffer | Variable $@ may not be null terminated. | test.cpp:326:8:326:13 | buffer | buffer |
21+
| test.cpp:346:11:346:16 | buffer | Variable $@ may not be null terminated. | test.cpp:341:8:341:13 | buffer | buffer |
22+
| test.cpp:355:11:355:16 | buffer | Variable $@ may not be null terminated. | test.cpp:350:8:350:13 | buffer | buffer |
23+
| test.cpp:365:19:365:25 | buffer2 | Variable $@ may not be null terminated. | test.cpp:363:8:363:14 | buffer2 | buffer2 |
24+
| test.cpp:392:17:392:22 | buffer | Variable $@ may not be null terminated. | test.cpp:390:8:390:13 | buffer | buffer |
25+
| test.cpp:398:18:398:23 | buffer | Variable $@ may not be null terminated. | test.cpp:396:8:396:13 | buffer | buffer |
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| test.cpp:410:10:410:15 | buffer | $@ flows to here and may not be null terminated. | test.cpp:409:18:409:23 | buffer | User-provided value |
2+
| test.cpp:425:10:425:15 | buffer | $@ flows to here and may not be null terminated. | test.cpp:424:9:424:14 | buffer | User-provided value |
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-170/ImproperNullTerminationTainted.ql

cpp/ql/test/query-tests/Likely Bugs/Memory Management/ImproperNullTermination/test.cpp

Lines changed: 228 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
1-
21
typedef unsigned int size_t;
32
typedef signed int ssize_t;
3+
typedef struct {} FILE;
44

55
size_t strlen(const char *s);
66
char *strcpy(char *s1, const char *s2);
7+
char *strcat(char *s1, const char *s2);
78
char *strdup(const char *s1);
8-
99
void *malloc(size_t size);
10-
1110
void *memset(void *s, int c, size_t n);
1211
void *memcpy(void *s1, const void *s2, size_t n);
13-
12+
void read(int src, void *out, int num);
13+
size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream);
1414
ssize_t readlink(const char *path, char *buffer, size_t buffer_size);
1515
ssize_t readlinkat(int fd, const char *path, char *buffer, size_t buffer_size);
1616

@@ -154,6 +154,18 @@ void test_readlink(int fd, const char *path, size_t sz)
154154
strdup(buffer); // GOOD
155155
}
156156

157+
{
158+
char buffer[1024];
159+
ssize_t len;
160+
161+
len = readlink(path, buffer, sizeof(buffer));
162+
if (len >= 0)
163+
{
164+
buffer[len - 1] = 0;
165+
strdup(buffer); // GOOD
166+
}
167+
}
168+
157169
{
158170
char buffer[1024];
159171

@@ -209,3 +221,215 @@ void test_readlink(int fd, const char *path, size_t sz)
209221
strdup(buffer); // GOOD
210222
}
211223
}
224+
225+
void doNothing(char *data) { };
226+
void doNothing2(const char *data);
227+
void clearBuffer(char *data, size_t size);
228+
229+
void test_strcat()
230+
{
231+
{
232+
char buffer[1024];
233+
234+
strcat(buffer, "content"); // BAD
235+
}
236+
237+
{
238+
char buffer[1024];
239+
240+
buffer[0] = 0;
241+
strcat(buffer, "content"); // GOOD
242+
}
243+
244+
{
245+
char buffer[1024];
246+
247+
buffer[10] = 0;
248+
strcat(buffer, "content"); // GOOD
249+
}
250+
251+
{
252+
char buffer[1024];
253+
254+
buffer[0] = '\0';
255+
strcat(buffer, "content"); // GOOD
256+
}
257+
258+
{
259+
char buffer[1024];
260+
261+
buffer[0] = 'a';
262+
strcat(buffer, "content"); // BAD
263+
}
264+
265+
{
266+
char buffer[1024];
267+
268+
*buffer = 0;
269+
strcat(buffer, "content"); // GOOD
270+
}
271+
272+
{
273+
char buffer[1024];
274+
275+
strcpy(buffer, "con");
276+
strcat(buffer, "tent"); // GOOD
277+
}
278+
279+
{
280+
char buffer[1024];
281+
282+
doNothing(buffer);
283+
strcat(buffer, "content"); // BAD
284+
}
285+
286+
{
287+
char buffer[1024];
288+
289+
doNothing2(buffer);
290+
strcat(buffer, "content"); // BAD [NOT DETECTED]
291+
}
292+
293+
{
294+
char buffer1[1024];
295+
char buffer2[1024];
296+
char *buffer_ptr = buffer1;
297+
298+
*buffer_ptr = 0;
299+
strcat(buffer1, "content"); // GOOD
300+
strcat(buffer2, "content"); // BAD
301+
strcat(buffer_ptr, "content"); // GOOD
302+
303+
buffer_ptr = buffer2;
304+
strcat(buffer_ptr, "content"); // BAD [NOT DETECTED]
305+
}
306+
307+
{
308+
char buffer[1024];
309+
char *buffer_ptr = buffer;
310+
311+
*buffer_ptr = 'a';
312+
strcat(buffer, "content"); // BAD
313+
}
314+
315+
{
316+
char buffer[1024];
317+
318+
clearBuffer(buffer, 1024);
319+
strcat(buffer, "content"); // GOOD
320+
}
321+
}
322+
323+
void test_strlen(bool cond1, bool cond2)
324+
{
325+
{
326+
char buffer[1024];
327+
int i = strlen(buffer); // BAD
328+
}
329+
330+
{
331+
char buffer[1024] = {0};
332+
int i = strlen(buffer); // GOOD
333+
}
334+
335+
{
336+
char *ptr = "content";
337+
int i = strlen(ptr); // GOOD
338+
}
339+
340+
{
341+
char buffer[1024];
342+
343+
if (cond1)
344+
buffer[0] = 0;
345+
if (cond1)
346+
strlen(buffer); // GOOD [FALSE POSITIVE]
347+
}
348+
349+
{
350+
char buffer[1024];
351+
352+
if (cond1)
353+
buffer[0] = 0;
354+
if (cond2)
355+
strlen(buffer); // BAD
356+
}
357+
}
358+
359+
void test_strcpy()
360+
{
361+
{
362+
char buffer1[1024];
363+
char buffer2[1024];
364+
365+
strcpy(buffer1, buffer2); // BAD
366+
}
367+
368+
{
369+
char buffer1[1024];
370+
char buffer2[1024];
371+
372+
strcpy(buffer2, "content"); // GOOD
373+
strcpy(buffer1, buffer2); // GOOD
374+
}
375+
}
376+
377+
void strcatWrapper(char *data, const char *with)
378+
{
379+
strcat(data, with);
380+
}
381+
382+
void strcatWrapper2(char *data, const char *with)
383+
{
384+
strcatWrapper(data, with);
385+
}
386+
387+
void test_wrappers()
388+
{
389+
{
390+
char buffer[1024];
391+
392+
strcatWrapper(buffer, "content"); // BAD
393+
}
394+
395+
{
396+
char buffer[1024];
397+
398+
strcatWrapper2(buffer, "content"); // BAD
399+
}
400+
}
401+
402+
void test_read_fread(int read_src, FILE *s)
403+
{
404+
const size_t buffer_size = 80;
405+
406+
{
407+
char buffer[buffer_size];
408+
409+
read(read_src, buffer, buffer_size * sizeof(char));
410+
strlen(buffer); // BAD
411+
}
412+
413+
{
414+
char buffer[buffer_size];
415+
416+
read(read_src, buffer, buffer_size * sizeof(char));
417+
buffer[buffer_size - 1] = 0;
418+
strlen(buffer); // GOOD
419+
}
420+
421+
{
422+
char buffer[buffer_size];
423+
424+
fread(buffer, sizeof(char), buffer_size, s);
425+
strlen(buffer); // BAD
426+
}
427+
428+
{
429+
char buffer[buffer_size];
430+
431+
fread(buffer, sizeof(char), buffer_size, s);
432+
buffer[buffer_size - 1] = 0;
433+
strlen(buffer); // GOOD
434+
}
435+
}

0 commit comments

Comments
 (0)