Revert "Revert "[analyzer] Fix taint rule of fgets and setproctitle_init""

This reverts commit 2acead35c1.

Let's try `REQUIRES: asserts`.
This commit is contained in:
Balazs Benics 2022-02-23 12:55:31 +01:00
parent a848a5cf2f
commit 7036413dc2
4 changed files with 22 additions and 10 deletions

View File

@ -559,7 +559,7 @@ void GenericTaintChecker::initTaintRules(CheckerContext &C) const {
{{"atoll"}, TR::Prop({{0}}, {{ReturnValueIndex}})}, {{"atoll"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
{{"fgetc"}, TR::Prop({{0}}, {{ReturnValueIndex}})}, {{"fgetc"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
{{"fgetln"}, TR::Prop({{0}}, {{ReturnValueIndex}})}, {{"fgetln"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
{{"fgets"}, TR::Prop({{2}}, {{0}, ReturnValueIndex})}, {{"fgets"}, TR::Prop({{2}}, {{0, ReturnValueIndex}})},
{{"fscanf"}, TR::Prop({{0}}, {{}, 2})}, {{"fscanf"}, TR::Prop({{0}}, {{}, 2})},
{{"sscanf"}, TR::Prop({{0}}, {{}, 2})}, {{"sscanf"}, TR::Prop({{0}}, {{}, 2})},
{{"getc"}, TR::Prop({{0}}, {{ReturnValueIndex}})}, {{"getc"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
@ -632,7 +632,7 @@ void GenericTaintChecker::initTaintRules(CheckerContext &C) const {
if (TR::UntrustedEnv(C)) { if (TR::UntrustedEnv(C)) {
// void setproctitle_init(int argc, char *argv[], char *envp[]) // void setproctitle_init(int argc, char *argv[], char *envp[])
GlobalCRules.push_back( GlobalCRules.push_back(
{{{"setproctitle_init"}}, TR::Sink({{2}}, MsgCustomSink)}); {{{"setproctitle_init"}}, TR::Sink({{1, 2}}, MsgCustomSink)});
GlobalCRules.push_back({{"getenv"}, TR::Source({{ReturnValueIndex}})}); GlobalCRules.push_back({{"getenv"}, TR::Source({{ReturnValueIndex}})});
} }

View File

@ -27,11 +27,9 @@ void top(const char *fname, char *buf) {
(void)fgets(buf, 42, fp); // Trigger taint propagation. (void)fgets(buf, 42, fp); // Trigger taint propagation.
// CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: -1 // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: -1
// CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 0 // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 0
// CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 1
// CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 2 // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 2
//
// CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: -1 // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: -1
// CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 0 // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 0
// CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 1
// CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 2 // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 2
} }

View File

@ -21,16 +21,11 @@ void top(const char *fname, char *buf) {
(void)fgets(buf, 42, fp); // Trigger taint propagation. (void)fgets(buf, 42, fp); // Trigger taint propagation.
// FIXME: Why is the arg index 1 prepared for taint?
// Before the call it wasn't tainted, and it also shouldn't be tainted after the call.
// CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: -1 // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: -1
// CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 0 // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 0
// CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 1
// CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 2 // CHECK-NEXT: PreCall<fgets(buf, 42, fp)> prepares tainting arg index: 2
// //
// CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: -1 // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: -1
// CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 0 // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 0
// CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 1
// CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 2 // CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 2
} }

View File

@ -58,9 +58,11 @@ extern FILE *stdin;
#define bool _Bool #define bool _Bool
char *getenv(const char *name);
int fscanf(FILE *restrict stream, const char *restrict format, ...); int fscanf(FILE *restrict stream, const char *restrict format, ...);
int sprintf(char *str, const char *format, ...); int sprintf(char *str, const char *format, ...);
void setproctitle(const char *fmt, ...); void setproctitle(const char *fmt, ...);
void setproctitle_init(int argc, char *argv[], char *envp[]);
typedef __typeof(sizeof(int)) size_t; typedef __typeof(sizeof(int)) size_t;
// Define string functions. Use builtin for some of them. They all default to // Define string functions. Use builtin for some of them. They all default to
@ -404,3 +406,20 @@ void testConfigurationSinks(void) {
void testUnknownFunction(void (*foo)(void)) { void testUnknownFunction(void (*foo)(void)) {
foo(); // no-crash foo(); // no-crash
} }
void testProctitleFalseNegative() {
char flag[80];
fscanf(stdin, "%79s", flag);
char *argv[] = {"myapp", flag};
// FIXME: We should have a warning below: Untrusted data passed to sink.
setproctitle_init(1, argv, 0);
}
void testProctitle2(char *real_argv[]) {
char *app = getenv("APP_NAME");
if (!app)
return;
char *argv[] = {app, "--foobar"};
setproctitle_init(1, argv, 0); // expected-warning {{Untrusted data is passed to a user-defined sink}}
setproctitle_init(1, real_argv, argv); // expected-warning {{Untrusted data is passed to a user-defined sink}}
}