[analyzer][taint] Extent of heap regions should get taint sometimes

[steakhal]

Summary

If we allocate a tainted amount of memory, the extent of that region will clearly depend on a tainted value.

We could later make use of this, for example when we try to prove that the array access is valid (0 <= idx < extent).
If the inequality would depend on the tainted value, we should still emit a warning (in ArrayBoundV2), as we currently do if the idx is tainted.

[steakhal]

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

[martong]

I like it, looks good to me!

[NoQ]

This will be obsoleted by D69726 because they are going to be the same symbol from the start.

[steakhal]

Obsoleted by D69726.

This effort continues as the NFC D99959 patch.

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

 //  InnerPointerChecker interacts very closely with MallocChecker, but unlike
 //  the above checkers, it has it's own file, hence the many InnerPointerChecker
 //  related headers and non-static functions.
 //
 //===----------------------------------------------------------------------===//
 
 #include "AllocationState.h"
 #include "InterCheckerAPI.h"
+#include "Taint.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
   if (Optional<DefinedOrUnknownSVal> DefinedSize =
           Size.getAs<DefinedOrUnknownSVal>()) {
     DefinedOrUnknownSVal DynSize = getDynamicSize(State, R, svalBuilder);
 
     DefinedOrUnknownSVal DynSizeMatchesSize =
         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);
   }
 
   return MallocUpdateRefState(C, CE, State, Family);
 }
 
 static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
                                             ProgramStateRef State,

clang/test/Analysis/malloc.c

 // RUN: %clang_analyze_cc1 -analyzer-store=region -verify %s \
 // RUN:   -analyzer-checker=core \
 // 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
 // the builtin type: "Using the typedef version can cause portability
 // problems", but we're ok here because we're not actually running anything.
 // Also of note is this cryptic warning: "The wchar_t type is not supported
 // when you compile C code".
 //

 void free(void *);
 void *realloc(void *ptr, size_t size);
 void *reallocf(void *ptr, size_t size);
 void *calloc(size_t nmemb, size_t size);
 char *strdup(const char *s);
 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);
 wchar_t *_wcsdup(const wchar_t *strSource);
 void *_alloca(size_t size);
 
 void myfoo(int *p);
 void myfooint(int p);
void testPassToSystemHeaderFunctionIndirectly() {
   // won't free (p-1) either.
 }
 
 void testMallocIntoMalloc() {
   StructWithPtr *s = malloc(sizeof(StructWithPtr));
   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@-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@-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@-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);
+  }
+}