There was a typo in the rule.
`{{0}, ReturnValueIndex}` meant that the discrete index is `0` and the
variadic index is `-1`.
What we wanted instead is that both `0` and `-1` are in the discrete index
list.
Instead of this, we wanted to express that both `0` and the
`ReturnValueIndex` is in the discrete arg list.
The manual inspection revealed that `setproctitle_init` also suffered a
probably incomplete propagation rule.
Reviewed By: Szelethus, gamesh411
Differential Revision: https://reviews.llvm.org/D119129
{{"atoll"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
{{"fgetc"}, 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})},
{{"sscanf"}, TR::Prop({{0}}, {{}, 2})},
{{"getc"}, TR::Prop({{0}}, {{ReturnValueIndex}})},
if (TR::UntrustedEnv(C)) {
// void setproctitle_init(int argc, char *argv[], char *envp[])
GlobalCRules.push_back(
- {{{"setproctitle_init"}}, TR::Sink({{2}}, MsgCustomSink)});
+ {{{"setproctitle_init"}}, TR::Sink({{1, 2}}, MsgCustomSink)});
GlobalCRules.push_back({{"getenv"}, TR::Source({{ReturnValueIndex}})});
}
(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: 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: 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: 1
// CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 2
}
(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: 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: 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: 1
// CHECK-NEXT: PostCall<fgets(buf, 42, fp)> actually wants to taint arg index: 2
}
#define bool _Bool
+char *getenv(const char *name);
int fscanf(FILE *restrict stream, const char *restrict format, ...);
int sprintf(char *str, const char *format, ...);
void setproctitle(const char *fmt, ...);
+void setproctitle_init(int argc, char *argv[], char *envp[]);
typedef __typeof(sizeof(int)) size_t;
// Define string functions. Use builtin for some of them. They all default to
void testUnknownFunction(void (*foo)(void)) {
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}}
+}