Skip to content

Commit 34a7387

Browse files
author
Endre Fülöp
committed
[analyzer] Add more sources to Taint analysis
Add more functions as taint sources to GenericTaintChecker. Reviewed By: steakhal Differential Revision: https://reviews.llvm.org/D120236
1 parent a44c984 commit 34a7387

File tree

3 files changed

+133
-5
lines changed

3 files changed

+133
-5
lines changed

clang/docs/analyzer/checkers.rst

+1-1
Original file line numberDiff line numberDiff line change
@@ -2355,7 +2355,7 @@ There are built-in sources, propagations and sinks defined in code inside ``Gene
23552355
These operations are handled even if no external taint configuration is provided.
23562356
23572357
Default sources defined by ``GenericTaintChecker``:
2358-
``fdopen``, ``fopen``, ``freopen``, ``getch``, ``getchar``, ``getchar_unlocked``, ``gets``, ``scanf``, ``socket``, ``wgetch``
2358+
``_IO_getc``, ``fdopen``, ``fopen``, ``freopen``, ``get_current_dir_name``, ``getch``, ``getchar``, ``getchar_unlocked``, ``getwd``, ``getcwd``, ``getgroups``, ``gethostname``, ``getlogin``, ``getlogin_r``, ``getnameinfo``, ``gets``, ``gets_s``, ``getseuserbyname``, ``readlink``, ``readlinkat``, ``scanf``, ``scanf_s``, ``socket``, ``wgetch``
23592359
23602360
Default propagations defined by ``GenericTaintChecker``:
23612361
``atoi``, ``atol``, ``atoll``, ``fgetc``, ``fgetln``, ``fgets``, ``fscanf``, ``sscanf``, ``getc``, ``getc_unlocked``, ``getdelim``, ``getline``, ``getw``, ``pread``, ``read``, ``strchr``, ``strrchr``, ``tolower``, ``toupper``

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

+19
Original file line numberDiff line numberDiff line change
@@ -550,8 +550,27 @@ void GenericTaintChecker::initTaintRules(CheckerContext &C) const {
550550
{{"getchar"}, TR::Source({{ReturnValueIndex}})},
551551
{{"getchar_unlocked"}, TR::Source({{ReturnValueIndex}})},
552552
{{"gets"}, TR::Source({{0}, ReturnValueIndex})},
553+
{{"gets_s"}, TR::Source({{0}, ReturnValueIndex})},
553554
{{"scanf"}, TR::Source({{}, 1})},
555+
{{"scanf_s"}, TR::Source({{}, {1}})},
554556
{{"wgetch"}, TR::Source({{}, ReturnValueIndex})},
557+
// Sometimes the line between taint sources and propagators is blurry.
558+
// _IO_getc is choosen to be a source, but could also be a propagator.
559+
// This way it is simpler, as modeling it as a propagator would require
560+
// to model the possible sources of _IO_FILE * values, which the _IO_getc
561+
// function takes as parameters.
562+
{{"_IO_getc"}, TR::Source({{ReturnValueIndex}})},
563+
{{"getcwd"}, TR::Source({{0, ReturnValueIndex}})},
564+
{{"getwd"}, TR::Source({{0, ReturnValueIndex}})},
565+
{{"readlink"}, TR::Source({{1, ReturnValueIndex}})},
566+
{{"readlinkat"}, TR::Source({{2, ReturnValueIndex}})},
567+
{{"get_current_dir_name"}, TR::Source({{ReturnValueIndex}})},
568+
{{"gethostname"}, TR::Source({{0}})},
569+
{{"getnameinfo"}, TR::Source({{2, 4}})},
570+
{{"getseuserbyname"}, TR::Source({{1, 2}})},
571+
{{"getgroups"}, TR::Source({{1, ReturnValueIndex}})},
572+
{{"getlogin"}, TR::Source({{ReturnValueIndex}})},
573+
{{"getlogin_r"}, TR::Source({{0}})},
555574

556575
// Props
557576
{{"atoi"}, TR::Prop({{0}}, {{ReturnValueIndex}})},

clang/test/Analysis/taint-generic.c

+113-4
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,11 @@
4545
// CHECK-INVALID-ARG-SAME: that expects an argument number for propagation
4646
// CHECK-INVALID-ARG-SAME: rules greater or equal to -1
4747

48+
typedef long long rsize_t;
49+
4850
int scanf(const char *restrict format, ...);
4951
char *gets(char *str);
52+
char *gets_s(char *str, rsize_t n);
5053
int getchar(void);
5154

5255
typedef struct _FILE FILE;
@@ -197,6 +200,12 @@ void testGets(void) {
197200
system(str); // expected-warning {{Untrusted data is passed to a system call}}
198201
}
199202

203+
void testGets_s(void) {
204+
char str[50];
205+
gets_s(str, 49);
206+
system(str); // expected-warning {{Untrusted data is passed to a system call}}
207+
}
208+
200209
void testTaintedBufferSize(void) {
201210
size_t ts;
202211
scanf("%zd", &ts);
@@ -343,15 +352,115 @@ void constraintManagerShouldTreatAsOpaque(int rhs) {
343352
*(volatile int *) 0; // no-warning
344353
}
345354

346-
int sprintf_is_not_a_source(char *buf, char *msg) {
355+
int testSprintf_is_not_a_source(char *buf, char *msg) {
347356
int x = sprintf(buf, "%s", msg); // no-warning
348-
return 1 / x; // no-warning: 'sprintf' is not a taint source
357+
return 1 / x; // no-warning: 'sprintf' is not a taint source
349358
}
350359

351-
int sprintf_propagates_taint(char *buf, char *msg) {
360+
int testSprintf_propagates_taint(char *buf, char *msg) {
352361
scanf("%s", msg);
353362
int x = sprintf(buf, "%s", msg); // propagate taint!
354-
return 1 / x; // expected-warning {{Division by a tainted value, possibly zero}}
363+
return 1 / x; // expected-warning {{Division by a tainted value, possibly zero}}
364+
}
365+
366+
int scanf_s(const char *format, ...);
367+
int testScanf_s_(int *out) {
368+
scanf_s("%d", out);
369+
return 1 / *out; // expected-warning {{Division by a tainted value, possibly zero}}
370+
}
371+
372+
#define _IO_FILE FILE
373+
int _IO_getc(_IO_FILE *__fp);
374+
int testUnderscoreIO_getc(_IO_FILE *fp) {
375+
char c = _IO_getc(fp);
376+
return 1 / c; // expected-warning {{Division by a tainted value, possibly zero}}
377+
}
378+
379+
char *getcwd(char *buf, size_t size);
380+
int testGetcwd(char *buf, size_t size) {
381+
char *c = getcwd(buf, size);
382+
return system(c); // expected-warning {{Untrusted data is passed to a system call}}
383+
}
384+
385+
char *getwd(char *buf);
386+
int testGetwd(char *buf) {
387+
char *c = getwd(buf);
388+
return system(c); // expected-warning {{Untrusted data is passed to a system call}}
389+
}
390+
391+
typedef signed long long ssize_t;
392+
ssize_t readlink(const char *path, char *buf, size_t bufsiz);
393+
int testReadlink(char *path, char *buf, size_t bufsiz) {
394+
ssize_t s = readlink(path, buf, bufsiz);
395+
system(buf); // expected-warning {{Untrusted data is passed to a system call}}
396+
// readlink never returns 0
397+
return 1 / (s + 1); // expected-warning {{Division by a tainted value, possibly zero}}
398+
}
399+
400+
ssize_t readlinkat(int dirfd, const char *pathname, char *buf, size_t bufsiz);
401+
int testReadlinkat(int dirfd, char *path, char *buf, size_t bufsiz) {
402+
ssize_t s = readlinkat(dirfd, path, buf, bufsiz);
403+
system(buf); // expected-warning {{Untrusted data is passed to a system call}}
404+
(void)(1 / dirfd); // arg 0 is not tainted
405+
system(path); // arg 1 is not tainted
406+
(void)(1 / bufsiz); // arg 3 is not tainted
407+
// readlinkat never returns 0
408+
return 1 / (s + 1); // expected-warning {{Division by a tainted value, possibly zero}}
409+
}
410+
411+
char *get_current_dir_name(void);
412+
int testGet_current_dir_name() {
413+
char *d = get_current_dir_name();
414+
return system(d); // expected-warning {{Untrusted data is passed to a system call}}
415+
}
416+
417+
int gethostname(char *name, size_t len);
418+
int testGethostname(char *name, size_t len) {
419+
gethostname(name, len);
420+
return system(name); // expected-warning {{Untrusted data is passed to a system call}}
421+
}
422+
423+
struct sockaddr;
424+
typedef size_t socklen_t;
425+
int getnameinfo(const struct sockaddr *restrict addr, socklen_t addrlen,
426+
char *restrict host, socklen_t hostlen,
427+
char *restrict serv, socklen_t servlen, int flags);
428+
int testGetnameinfo(const struct sockaddr *restrict addr, socklen_t addrlen,
429+
char *restrict host, socklen_t hostlen,
430+
char *restrict serv, socklen_t servlen, int flags) {
431+
getnameinfo(addr, addrlen, host, hostlen, serv, servlen, flags);
432+
433+
system(host); // expected-warning {{Untrusted data is passed to a system call}}
434+
return system(serv); // expected-warning {{Untrusted data is passed to a system call}}
435+
}
436+
437+
int getseuserbyname(const char *linuxuser, char **selinuxuser, char **level);
438+
int testGetseuserbyname(const char *linuxuser, char **selinuxuser, char **level) {
439+
getseuserbyname(linuxuser, selinuxuser, level);
440+
system(selinuxuser[0]); // expected-warning {{Untrusted data is passed to a system call}}
441+
return system(level[0]); // expected-warning {{Untrusted data is passed to a system call}}
442+
}
443+
444+
typedef int gid_t;
445+
int getgroups(int size, gid_t list[]);
446+
int testGetgroups(int size, gid_t list[], bool flag) {
447+
int result = getgroups(size, list);
448+
if (flag)
449+
return 1 / list[0]; // expected-warning {{Division by a tainted value, possibly zero}}
450+
451+
return 1 / (result + 1); // expected-warning {{Division by a tainted value, possibly zero}}
452+
}
453+
454+
char *getlogin(void);
455+
int testGetlogin() {
456+
char *n = getlogin();
457+
return system(n); // expected-warning {{Untrusted data is passed to a system call}}
458+
}
459+
460+
int getlogin_r(char *buf, size_t bufsize);
461+
int testGetlogin_r(char *buf, size_t bufsize) {
462+
getlogin_r(buf, bufsize);
463+
return system(buf); // expected-warning {{Untrusted data is passed to a system call}}
355464
}
356465

357466
// Test configuration

0 commit comments

Comments
 (0)