[PATCH] D99659: [analyzer][taint] Extent of heap regions should get taint sometimes

[Balázs Benics via Phabricator via cfe-commits]

steakhal updated this revision to Diff 334474.
steakhal added a comment.

Add a FIXME about placing a NoteTag describing why the extent was getting tainted.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99659/new/

https://reviews.llvm.org/D99659

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/malloc.c


Index: clang/test/Analysis/malloc.c
===================================================================
--- clang/test/Analysis/malloc.c
+++ clang/test/Analysis/malloc.c
@@ -3,11 +3,14 @@
 // RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
 // RUN:   -analyzer-checker=alpha.core.CastSize \
 // RUN:   -analyzer-checker=unix \
+// RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-checker=debug.ExprInspection
 
 #include "Inputs/system-header-simulator.h"
 
 void clang_analyzer_eval(int);
+size_t clang_analyzer_getExtent(void *);
+void clang_analyzer_isTainted(int);
 
 // Without -fms-compatibility, wchar_t isn't a builtin type. MSVC defines
 // _WCHAR_T_DEFINED if wchar_t is available. Microsoft recommends that you use
@@ -36,6 +39,7 @@
 wchar_t *wcsdup(const wchar_t *s);
 char *strndup(const char *s, size_t n);
 int memcmp(const void *s1, const void *s2, size_t n);
+int getchar(void);
 
 // Windows variants
 char *_strdup(const char *strSource);
@@ -1883,3 +1887,36 @@
   s->memP = malloc(sizeof(int));
   free(s);
 } // FIXME: should warn here
+
+void extentGetsTainted(int conj) {
+  int tainted = getchar();
+  {
+    int *p = (int *)malloc(conj);
+    clang_analyzer_isTainted(clang_analyzer_getExtent(p)); // expected-warning {{NO}}
+    free(p);
+  }
+  {
+    int *p = (int *)malloc(tainted);
+    // expected-warning at -1 {{Untrusted data is used to specify the buffer size \
+(CERT/STR31-C. Guarantee that storage for strings has sufficient space for \
+character data and the null terminator)}}
+    clang_analyzer_isTainted(clang_analyzer_getExtent(p)); // expected-warning {{YES}}
+    free(p);
+  }
+  {
+    int *p = (int *)malloc(5 + tainted);
+    // expected-warning at -1 {{Untrusted data is used to specify the buffer size \
+(CERT/STR31-C. Guarantee that storage for strings has sufficient space for \
+character data and the null terminator)}}
+    clang_analyzer_isTainted(clang_analyzer_getExtent(p)); // expected-warning {{YES}}
+    free(p);
+  }
+  {
+    int *p = (int *)malloc(conj + tainted);
+    // expected-warning at -1 {{Untrusted data is used to specify the buffer size \
+(CERT/STR31-C. Guarantee that storage for strings has sufficient space for \
+character data and the null terminator)}}
+    clang_analyzer_isTainted(clang_analyzer_getExtent(p)); // expected-warning {{YES}}
+    free(p);
+  }
+}
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -46,6 +46,7 @@
 
 #include "AllocationState.h"
 #include "InterCheckerAPI.h"
+#include "Taint.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
@@ -1601,6 +1602,12 @@
         svalBuilder.evalEQ(State, DynSize, *DefinedSize);
 
     State = State->assume(DynSizeMatchesSize, true);
+
+    // If the size of the allocation is tainted, the associated extent should be
+    // also tainted.
+    // FIXME: Add a NoteTag to describe why the extent become tainted.
+    if (taint::isTainted(State, Size))
+      State = taint::addTaint(State, DynSize);
     assert(State);
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D99659.334474.patch
Type: text/x-patch
Size: 3232 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210331/b96adee0/attachment.bin>