[PATCH] D58828: [analyzer] Fix taint propagation in GenericTaintChecker

[Borsik Gábor via Phabricator via cfe-commits]

boga95 created this revision.
boga95 added reviewers: gerazo, xazax.hun, Szelethus, a_sidorin, dcoughlin, 
george.karpenkov, NoQ.
boga95 added a project: clang.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity.

The `gets` function has no SrcArgs. Because the default value for isTainted was 
false, it didn't mark its DstArgs as tainted.


Repository:
  rC Clang

https://reviews.llvm.org/D58828

Files:
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  test/Analysis/taint-generic.c


Index: test/Analysis/taint-generic.c
===================================================================
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -2,6 +2,7 @@
 // RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT 
-analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 
-Wno-format-security -verify %s
 
 int scanf(const char *restrict format, ...);
+char *gets(char *str);
 int getchar(void);
 
 typedef struct _FILE FILE;
@@ -142,6 +143,12 @@
   system(buffern2); // expected-warning {{Untrusted data is passed to a system 
call}}
 }
 
+void testGets() {
+  char str[50];
+  gets(str);
+  system(str); // expected-warning {{Untrusted data is passed to a system 
call}}
+}
+
 void testTaintedBufferSize() {
   size_t ts;
   scanf("%zd", &ts);
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -458,7 +458,7 @@
   ProgramStateRef State = C.getState();
 
   // Check for taint in arguments.
-  bool IsTainted = false;
+  bool IsTainted = true;
   for (unsigned ArgNum : SrcArgs) {
     if (ArgNum >= CE->getNumArgs())
       return State;



Index: test/Analysis/taint-generic.c
===================================================================
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -2,6 +2,7 @@
 // RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -Wno-format-security -verify %s
 
 int scanf(const char *restrict format, ...);
+char *gets(char *str);
 int getchar(void);
 
 typedef struct _FILE FILE;
@@ -142,6 +143,12 @@
   system(buffern2); // expected-warning {{Untrusted data is passed to a system call}}
 }
 
+void testGets() {
+  char str[50];
+  gets(str);
+  system(str); // expected-warning {{Untrusted data is passed to a system call}}
+}
+
 void testTaintedBufferSize() {
   size_t ts;
   scanf("%zd", &ts);
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -458,7 +458,7 @@
   ProgramStateRef State = C.getState();
 
   // Check for taint in arguments.
-  bool IsTainted = false;
+  bool IsTainted = true;
   for (unsigned ArgNum : SrcArgs) {
     if (ArgNum >= CE->getNumArgs())
       return State;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits