steakhal created this revision.
steakhal added reviewers: NoQ, vsavchenko, martong, balazske, xazax.hun.
Herald added subscribers: ASDenysPetrov, Charusso, dkrupp, donat.nagy,
Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware,
whisperity.
Herald added a reviewer: Szelethus.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
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@-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);
+ }
+}
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,11 @@
svalBuilder.evalEQ(State, DynSize, *DefinedSize);
State = State->assume(DynSizeMatchesSize, true);
+
+ // If the size of the allocation is tainted, the associated dynamic extent
+ // should be tainted as well.
+ if (taint::isTainted(State, Size))
+ State = taint::addTaint(State, DynSize);
assert(State);
}
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@-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);
+ }
+}
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,11 @@
svalBuilder.evalEQ(State, DynSize, *DefinedSize);
State = State->assume(DynSizeMatchesSize, true);
+
+ // If the size of the allocation is tainted, the associated dynamic extent
+ // should be tainted as well.
+ if (taint::isTainted(State, Size))
+ State = taint::addTaint(State, DynSize);
assert(State);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
