C++: comment and test for taint flow via memcpy

Robert Marsh · Robert Marsh · commit 919f5c616fe5 · 2019-04-23T11:17:18.000-07:00
diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/TaintTracking.qll b/cpp/ql/src/semmle/code/cpp/dataflow/TaintTracking.qll
@@ -239,6 +239,8 @@ module TaintTracking {
       exists(int argInIndex, FunctionInput inModel |
         f.hasDataFlow(inModel, outModel)
       |
+	// Taint flows from a pointer to a dereference, which DataFlow does not handle
+        // memcpy(&dest_var, tainted_ptr, len)
         inModel.isInParameterPointer(argInIndex) and
         exprIn = call.getArgument(argInIndex)
       )
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
@@ -137,15 +137,27 @@
 | taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:171:8:171:13 | buffer |  |
 | taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:172:10:172:15 | buffer |  |
 | taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
+| taint.cpp:170:18:170:26 | Hello,  | taint.cpp:170:10:170:15 | ref arg buffer | TAINT |
 | taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:171:8:171:13 | buffer |  |
 | taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:172:10:172:15 | buffer |  |
 | taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:172:10:172:15 | buffer | taint.cpp:172:3:172:8 | call to strcat |  |
+| taint.cpp:172:10:172:15 | buffer | taint.cpp:172:10:172:15 | ref arg buffer | TAINT |
 | taint.cpp:172:10:172:15 | ref arg buffer | taint.cpp:172:10:172:15 | buffer |  |
 | taint.cpp:172:10:172:15 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
+| taint.cpp:172:18:172:24 | tainted | taint.cpp:172:10:172:15 | ref arg buffer | TAINT |
 | taint.cpp:173:8:173:13 | ref arg buffer | taint.cpp:173:8:173:13 | buffer |  |
 | taint.cpp:180:19:180:19 | p | taint.cpp:181:9:181:9 | p |  |
 | taint.cpp:181:9:181:9 | p | taint.cpp:181:8:181:9 | * ... | TAINT |
 | taint.cpp:185:11:185:16 | call to source | taint.cpp:186:11:186:11 | x |  |
 | taint.cpp:186:10:186:11 | ref arg & ... | taint.cpp:186:11:186:11 | x |  |
 | taint.cpp:186:11:186:11 | x | taint.cpp:186:10:186:11 | & ... | TAINT |
+| taint.cpp:192:23:192:28 | source | taint.cpp:194:13:194:18 | source |  |
+| taint.cpp:193:6:193:6 | x | taint.cpp:194:10:194:10 | x |  |
+| taint.cpp:193:6:193:6 | x | taint.cpp:195:7:195:7 | x |  |
+| taint.cpp:194:9:194:10 | & ... | taint.cpp:194:2:194:7 | call to memcpy |  |
+| taint.cpp:194:9:194:10 | ref arg & ... | taint.cpp:194:10:194:10 | x |  |
+| taint.cpp:194:9:194:10 | ref arg & ... | taint.cpp:195:7:195:7 | x |  |
+| taint.cpp:194:10:194:10 | x | taint.cpp:194:9:194:10 | & ... | TAINT |
+| taint.cpp:194:13:194:18 | source | taint.cpp:194:9:194:10 | ref arg & ... | TAINT |
+| taint.cpp:194:21:194:31 | sizeof(int) | taint.cpp:194:9:194:10 | ref arg & ... | TAINT |
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp
@@ -170,7 +170,7 @@ namespace strings
 		strcpy(buffer, "Hello, ");
 		sink(buffer);
 		strcat(buffer, tainted);
-		sink(buffer); // tainted [NOT DETECTED]
+		sink(buffer); // tainted
 	}
 }
 
@@ -186,3 +186,11 @@ namespace refs {
 		callee(&x);
 	}
 }
+
+void *memcpy(void *dest, void *src, int len);
+
+void test_memcpy(int *source) {
+	int x;
+	memcpy(&x, source, sizeof(int));
+	sink(x);
+}
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected
@@ -12,3 +12,5 @@
 | taint.cpp:168:8:168:14 | tainted | taint.cpp:164:19:164:24 | call to source |
 | taint.cpp:173:8:173:13 | buffer | taint.cpp:164:19:164:24 | call to source |
 | taint.cpp:181:8:181:9 | * ... | taint.cpp:185:11:185:16 | call to source |
+| taint.cpp:195:7:195:7 | x | taint.cpp:192:23:192:28 | source |
+| taint.cpp:195:7:195:7 | x | taint.cpp:193:6:193:6 | x |
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected
@@ -3,4 +3,7 @@
 | taint.cpp:37:22:37:27 | taint.cpp:43:7:43:13 | AST only |
 | taint.cpp:120:11:120:16 | taint.cpp:137:7:137:9 | AST only |
 | taint.cpp:127:8:127:13 | taint.cpp:130:7:130:9 | IR only |
+| taint.cpp:164:19:164:24 | taint.cpp:173:8:173:13 | AST only |
 | taint.cpp:185:11:185:16 | taint.cpp:181:8:181:9 | AST only |
+| taint.cpp:192:23:192:28 | taint.cpp:195:7:195:7 | AST only |
+| taint.cpp:193:6:193:6 | taint.cpp:195:7:195:7 | AST only |

Original file line number	Diff line number	Diff line change
`@@ -239,6 +239,8 @@ module TaintTracking {`
`239`	`239`	`exists(int argInIndex, FunctionInput inModel \|`
`240`	`240`	`f.hasDataFlow(inModel, outModel)`
`241`	`241`	`\|`
	`242`	`+ // Taint flows from a pointer to a dereference, which DataFlow does not handle`
	`243`	`+ // memcpy(&dest_var, tainted_ptr, len)`
`242`	`244`	`inModel.isInParameterPointer(argInIndex) and`
`243`	`245`	`exprIn = call.getArgument(argInIndex)`
`244`	`246`	`)`
Original file line number	Diff line number	Diff line change
`@@ -170,7 +170,7 @@ namespace strings`
`170`	`170`	`strcpy(buffer, "Hello, ");`
`171`	`171`	`sink(buffer);`
`172`	`172`	`strcat(buffer, tainted);`
`173`		`- sink(buffer); // tainted [NOT DETECTED]`
	`173`	`+ sink(buffer); // tainted`
`174`	`174`	`}`
`175`	`175`	`}`
`176`	`176`
`@@ -186,3 +186,11 @@ namespace refs {`
`186`	`186`	`callee(&x);`
`187`	`187`	`}`
`188`	`188`	`}`
	`189`	`+`
	`190`	`+void memcpy(void dest, void *src, int len);`
	`191`	`+`
	`192`	`+void test_memcpy(int *source) {`
	`193`	`+ int x;`
	`194`	`+ memcpy(&x, source, sizeof(int));`
	`195`	`+ sink(x);`
	`196`	`+}`