C++: Taint from FormattingFunction varargs.

geoffw0 · geoffw0 · commit d66f608d4153 · 2020-01-28T14:53:18.000Z
diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
@@ -154,7 +154,13 @@ abstract class FormattingFunction extends ArrayFunction, TaintFunction {
   override predicate hasArrayOutput(int bufParam) { bufParam = getOutputParameterIndex() }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    input.isParameterDeref(getFormatParameterIndex()) and
-    output.isParameterDeref(getOutputParameterIndex())
+    exists(int arg |
+      (
+        arg = getFormatParameterIndex() or
+        arg >= getFirstFormatArgumentIndex()
+      ) and
+      input.isParameterDeref(arg) and
+      output.isParameterDeref(getOutputParameterIndex())
+    )
   }
 }
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/format.cpp
@@ -50,7 +50,7 @@ void test1()
 	{
 		char buffer[256] = {0};
 		sink(snprintf(buffer, 256, "%s", string::source()));
-		sink(buffer); // tainted [NOT DETECTED]
+		sink(buffer); // tainted
 	}
 	{
 		char buffer[256] = {0};
@@ -76,34 +76,34 @@ void test1()
 	{
 		char buffer[256] = {0};
 		sink(snprintf(buffer, 256, "%i", source()));
-		sink(buffer); // tainted [NOT DETECTED]
+		sink(buffer); // tainted
 	}
 	{
 		char buffer[256] = {0};
 		sink(snprintf(buffer, 256, "%.*s", source(), "Hello."));
-		sink(buffer); // tainted [NOT DETECTED]
+		sink(buffer); // tainted
 	}
 
 	{
 		char buffer[256] = {0};
 		sink(snprintf(buffer, 256, "%p", string::source()));
-		sink(buffer);
+		sink(buffer); // tainted (debatable)
 	}
 
 	{
 		char buffer[256] = {0};
 		sink(sprintf(buffer, "%s", string::source()));
-		sink(buffer); // tainted [NOT DETECTED]
+		sink(buffer); // tainted
 	}
 	{
 		char buffer[256] = {0};
 		sink(sprintf(buffer, "%ls", wstring::source()));
-		sink(buffer); // tainted [NOT DETECTED]
+		sink(buffer); // tainted
 	}
 	{
 		wchar_t wbuffer[256] = {0};
 		sink(swprintf(wbuffer, 256, L"%s", wstring::source()));
-		sink(wbuffer); // tainted [NOT DETECTED]
+		sink(wbuffer); // tainted
 	}
 	{
 		char buffer[256] = {0};
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
@@ -12,61 +12,73 @@
 | format.cpp:46:23:46:23 | 0 | format.cpp:46:21:46:24 | {...} | TAINT |
 | format.cpp:47:17:47:22 | ref arg buffer | format.cpp:48:8:48:13 | buffer |  |
 | format.cpp:47:30:47:33 | %s | format.cpp:47:17:47:22 | ref arg buffer | TAINT |
+| format.cpp:47:36:47:43 | Hello. | format.cpp:47:17:47:22 | ref arg buffer | TAINT |
 | format.cpp:51:21:51:24 | {...} | format.cpp:52:17:52:22 | buffer |  |
 | format.cpp:51:21:51:24 | {...} | format.cpp:53:8:53:13 | buffer |  |
 | format.cpp:51:23:51:23 | 0 | format.cpp:51:21:51:24 | {...} | TAINT |
 | format.cpp:52:17:52:22 | ref arg buffer | format.cpp:53:8:53:13 | buffer |  |
 | format.cpp:52:30:52:33 | %s | format.cpp:52:17:52:22 | ref arg buffer | TAINT |
+| format.cpp:52:36:52:49 | call to source | format.cpp:52:17:52:22 | ref arg buffer | TAINT |
 | format.cpp:56:21:56:24 | {...} | format.cpp:57:17:57:22 | buffer |  |
 | format.cpp:56:21:56:24 | {...} | format.cpp:58:8:58:13 | buffer |  |
 | format.cpp:56:23:56:23 | 0 | format.cpp:56:21:56:24 | {...} | TAINT |
 | format.cpp:57:17:57:22 | ref arg buffer | format.cpp:58:8:58:13 | buffer |  |
 | format.cpp:57:30:57:43 | call to source | format.cpp:57:17:57:22 | ref arg buffer | TAINT |
+| format.cpp:57:48:57:55 | Hello. | format.cpp:57:17:57:22 | ref arg buffer | TAINT |
 | format.cpp:61:21:61:24 | {...} | format.cpp:62:17:62:22 | buffer |  |
 | format.cpp:61:21:61:24 | {...} | format.cpp:63:8:63:13 | buffer |  |
 | format.cpp:61:23:61:23 | 0 | format.cpp:61:21:61:24 | {...} | TAINT |
 | format.cpp:62:17:62:22 | ref arg buffer | format.cpp:63:8:63:13 | buffer |  |
 | format.cpp:62:30:62:39 | %s %s %s | format.cpp:62:17:62:22 | ref arg buffer | TAINT |
+| format.cpp:62:42:62:44 | a | format.cpp:62:17:62:22 | ref arg buffer | TAINT |
 | format.cpp:66:21:66:24 | {...} | format.cpp:67:17:67:22 | buffer |  |
 | format.cpp:66:21:66:24 | {...} | format.cpp:68:8:68:13 | buffer |  |
 | format.cpp:66:23:66:23 | 0 | format.cpp:66:21:66:24 | {...} | TAINT |
 | format.cpp:67:17:67:22 | ref arg buffer | format.cpp:68:8:68:13 | buffer |  |
 | format.cpp:67:30:67:35 | %.*s | format.cpp:67:17:67:22 | ref arg buffer | TAINT |
+| format.cpp:67:38:67:39 | 10 | format.cpp:67:17:67:22 | ref arg buffer | TAINT |
 | format.cpp:72:21:72:24 | {...} | format.cpp:73:17:73:22 | buffer |  |
 | format.cpp:72:21:72:24 | {...} | format.cpp:74:8:74:13 | buffer |  |
 | format.cpp:72:23:72:23 | 0 | format.cpp:72:21:72:24 | {...} | TAINT |
 | format.cpp:73:17:73:22 | ref arg buffer | format.cpp:74:8:74:13 | buffer |  |
 | format.cpp:73:30:73:33 | %i | format.cpp:73:17:73:22 | ref arg buffer | TAINT |
+| format.cpp:73:36:73:36 | 0 | format.cpp:73:17:73:22 | ref arg buffer | TAINT |
 | format.cpp:77:21:77:24 | {...} | format.cpp:78:17:78:22 | buffer |  |
 | format.cpp:77:21:77:24 | {...} | format.cpp:79:8:79:13 | buffer |  |
 | format.cpp:77:23:77:23 | 0 | format.cpp:77:21:77:24 | {...} | TAINT |
 | format.cpp:78:17:78:22 | ref arg buffer | format.cpp:79:8:79:13 | buffer |  |
 | format.cpp:78:30:78:33 | %i | format.cpp:78:17:78:22 | ref arg buffer | TAINT |
+| format.cpp:78:36:78:41 | call to source | format.cpp:78:17:78:22 | ref arg buffer | TAINT |
 | format.cpp:82:21:82:24 | {...} | format.cpp:83:17:83:22 | buffer |  |
 | format.cpp:82:21:82:24 | {...} | format.cpp:84:8:84:13 | buffer |  |
 | format.cpp:82:23:82:23 | 0 | format.cpp:82:21:82:24 | {...} | TAINT |
 | format.cpp:83:17:83:22 | ref arg buffer | format.cpp:84:8:84:13 | buffer |  |
 | format.cpp:83:30:83:35 | %.*s | format.cpp:83:17:83:22 | ref arg buffer | TAINT |
+| format.cpp:83:38:83:43 | call to source | format.cpp:83:17:83:22 | ref arg buffer | TAINT |
 | format.cpp:88:21:88:24 | {...} | format.cpp:89:17:89:22 | buffer |  |
 | format.cpp:88:21:88:24 | {...} | format.cpp:90:8:90:13 | buffer |  |
 | format.cpp:88:23:88:23 | 0 | format.cpp:88:21:88:24 | {...} | TAINT |
 | format.cpp:89:17:89:22 | ref arg buffer | format.cpp:90:8:90:13 | buffer |  |
 | format.cpp:89:30:89:33 | %p | format.cpp:89:17:89:22 | ref arg buffer | TAINT |
+| format.cpp:89:36:89:49 | call to source | format.cpp:89:17:89:22 | ref arg buffer | TAINT |
 | format.cpp:94:21:94:24 | {...} | format.cpp:95:16:95:21 | buffer |  |
 | format.cpp:94:21:94:24 | {...} | format.cpp:96:8:96:13 | buffer |  |
 | format.cpp:94:23:94:23 | 0 | format.cpp:94:21:94:24 | {...} | TAINT |
 | format.cpp:95:16:95:21 | ref arg buffer | format.cpp:96:8:96:13 | buffer |  |
 | format.cpp:95:24:95:27 | %s | format.cpp:95:16:95:21 | ref arg buffer | TAINT |
+| format.cpp:95:30:95:43 | call to source | format.cpp:95:16:95:21 | ref arg buffer | TAINT |
 | format.cpp:99:21:99:24 | {...} | format.cpp:100:16:100:21 | buffer |  |
 | format.cpp:99:21:99:24 | {...} | format.cpp:101:8:101:13 | buffer |  |
 | format.cpp:99:23:99:23 | 0 | format.cpp:99:21:99:24 | {...} | TAINT |
 | format.cpp:100:16:100:21 | ref arg buffer | format.cpp:101:8:101:13 | buffer |  |
 | format.cpp:100:24:100:28 | %ls | format.cpp:100:16:100:21 | ref arg buffer | TAINT |
+| format.cpp:100:31:100:45 | call to source | format.cpp:100:16:100:21 | ref arg buffer | TAINT |
 | format.cpp:104:25:104:28 | {...} | format.cpp:105:17:105:23 | wbuffer |  |
 | format.cpp:104:25:104:28 | {...} | format.cpp:106:8:106:14 | wbuffer |  |
 | format.cpp:104:27:104:27 | 0 | format.cpp:104:25:104:28 | {...} | TAINT |
 | format.cpp:105:17:105:23 | ref arg wbuffer | format.cpp:106:8:106:14 | wbuffer |  |
 | format.cpp:105:31:105:35 | %s | format.cpp:105:17:105:23 | ref arg wbuffer | TAINT |
+| format.cpp:105:38:105:52 | call to source | format.cpp:105:17:105:23 | ref arg wbuffer | TAINT |
 | format.cpp:109:21:109:24 | {...} | format.cpp:110:18:110:23 | buffer |  |
 | format.cpp:109:21:109:24 | {...} | format.cpp:111:8:111:13 | buffer |  |
 | format.cpp:109:23:109:23 | 0 | format.cpp:109:21:109:24 | {...} | TAINT |
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected
@@ -1,4 +1,11 @@
+| format.cpp:53:8:53:13 | buffer | format.cpp:52:36:52:49 | call to source |
 | format.cpp:58:8:58:13 | buffer | format.cpp:57:30:57:43 | call to source |
+| format.cpp:79:8:79:13 | buffer | format.cpp:78:36:78:41 | call to source |
+| format.cpp:84:8:84:13 | buffer | format.cpp:83:38:83:43 | call to source |
+| format.cpp:90:8:90:13 | buffer | format.cpp:89:36:89:49 | call to source |
+| format.cpp:96:8:96:13 | buffer | format.cpp:95:30:95:43 | call to source |
+| format.cpp:101:8:101:13 | buffer | format.cpp:100:31:100:45 | call to source |
+| format.cpp:106:8:106:14 | wbuffer | format.cpp:105:38:105:52 | call to source |
 | taint.cpp:8:8:8:13 | clean1 | taint.cpp:4:27:4:33 | source1 |
 | taint.cpp:16:8:16:14 | source1 | taint.cpp:12:22:12:27 | call to source |
 | taint.cpp:17:8:17:16 | ++ ... | taint.cpp:12:22:12:27 | call to source |
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
@@ -1,4 +1,11 @@
+| format.cpp:53:8:53:13 | format.cpp:52:36:52:49 | AST only |
 | format.cpp:58:8:58:13 | format.cpp:57:30:57:43 | AST only |
+| format.cpp:79:8:79:13 | format.cpp:78:36:78:41 | AST only |
+| format.cpp:84:8:84:13 | format.cpp:83:38:83:43 | AST only |
+| format.cpp:90:8:90:13 | format.cpp:89:36:89:49 | AST only |
+| format.cpp:96:8:96:13 | format.cpp:95:30:95:43 | AST only |
+| format.cpp:101:8:101:13 | format.cpp:100:31:100:45 | AST only |
+| format.cpp:106:8:106:14 | format.cpp:105:38:105:52 | AST only |
 | taint.cpp:41:7:41:13 | taint.cpp:35:12:35:17 | AST only |
 | taint.cpp:42:7:42:13 | taint.cpp:35:12:35:17 | AST only |
 | taint.cpp:43:7:43:13 | taint.cpp:37:22:37:27 | AST only |

Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ void test1()`
`50`	`50`	`{`
`51`	`51`	`char buffer[256] = {0};`
`52`	`52`	`sink(snprintf(buffer, 256, "%s", string::source()));`
`53`		`- sink(buffer); // tainted [NOT DETECTED]`
	`53`	`+ sink(buffer); // tainted`
`54`	`54`	`}`
`55`	`55`	`{`
`56`	`56`	`char buffer[256] = {0};`
`@@ -76,34 +76,34 @@ void test1()`
`76`	`76`	`{`
`77`	`77`	`char buffer[256] = {0};`
`78`	`78`	`sink(snprintf(buffer, 256, "%i", source()));`
`79`		`- sink(buffer); // tainted [NOT DETECTED]`
	`79`	`+ sink(buffer); // tainted`
`80`	`80`	`}`
`81`	`81`	`{`
`82`	`82`	`char buffer[256] = {0};`
`83`	`83`	`sink(snprintf(buffer, 256, "%.*s", source(), "Hello."));`
`84`		`- sink(buffer); // tainted [NOT DETECTED]`
	`84`	`+ sink(buffer); // tainted`
`85`	`85`	`}`
`86`	`86`
`87`	`87`	`{`
`88`	`88`	`char buffer[256] = {0};`
`89`	`89`	`sink(snprintf(buffer, 256, "%p", string::source()));`
`90`		`- sink(buffer);`
	`90`	`+ sink(buffer); // tainted (debatable)`
`91`	`91`	`}`
`92`	`92`
`93`	`93`	`{`
`94`	`94`	`char buffer[256] = {0};`
`95`	`95`	`sink(sprintf(buffer, "%s", string::source()));`
`96`		`- sink(buffer); // tainted [NOT DETECTED]`
	`96`	`+ sink(buffer); // tainted`
`97`	`97`	`}`
`98`	`98`	`{`
`99`	`99`	`char buffer[256] = {0};`
`100`	`100`	`sink(sprintf(buffer, "%ls", wstring::source()));`
`101`		`- sink(buffer); // tainted [NOT DETECTED]`
	`101`	`+ sink(buffer); // tainted`
`102`	`102`	`}`
`103`	`103`	`{`
`104`	`104`	`wchar_t wbuffer[256] = {0};`
`105`	`105`	`sink(swprintf(wbuffer, 256, L"%s", wstring::source()));`
`106`		`- sink(wbuffer); // tainted [NOT DETECTED]`
	`106`	`+ sink(wbuffer); // tainted`
`107`	`107`	`}`
`108`	`108`	`{`
`109`	`109`	`char buffer[256] = {0};`