Merge pull request github#14801 from jketema/rewrite-tainted-format-s…

…tring

C++: Rewrite `cpp/tainted-format-string` away from `DefaultTaintTracking`

cpp/ql/src/Security/CWE/CWE-134/UncontrolledFormatString.ql

@@ -16,22 +16,50 @@
 import cpp
 import semmle.code.cpp.security.Security
 import semmle.code.cpp.security.FunctionWithWrappers
-import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl
-import TaintedWithPath
+import semmle.code.cpp.security.FlowSources
+import semmle.code.cpp.ir.dataflow.TaintTracking
+import semmle.code.cpp.ir.IR
+import Flow::PathGraph
 
-class Configuration extends TaintTrackingConfiguration {
-  override predicate isSink(Element tainted) {
-    exists(PrintfLikeFunction printf | printf.outermostWrapperFunctionCall(tainted, _))
+predicate isSource(FlowSource source, string sourceType) {
+  not source instanceof DataFlow::ExprNode and
+  sourceType = source.getSourceType()
+}
+
+module Config implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { isSource(node, _) }
+
+  predicate isSink(DataFlow::Node node) {
+    exists(PrintfLikeFunction printf |
+      printf.outermostWrapperFunctionCall([node.asExpr(), node.asIndirectExpr()], _)
+    )
+  }
+
+  private predicate isArithmeticNonCharType(ArithmeticType type) {
+    not type instanceof CharType and
+    not type instanceof Char8Type and
+    not type instanceof Char16Type and
+    not type instanceof Char32Type
+  }
+
+  predicate isBarrier(DataFlow::Node node) {
+    isSink(node) and isArithmeticNonCharType(node.asExpr().getUnspecifiedType())
+    or
+    isArithmeticNonCharType(node.asInstruction().(StoreInstruction).getResultType())
   }
 }
 
+module Flow = TaintTracking::Global<Config>;
+
 from
-  PrintfLikeFunction printf, Expr arg, PathNode sourceNode, PathNode sinkNode,
-  string printfFunction, Expr userValue, string cause
+  PrintfLikeFunction printf, string printfFunction, string sourceType, DataFlow::Node source,
+  DataFlow::Node sink, Flow::PathNode sourceNode, Flow::PathNode sinkNode
 where
-  printf.outermostWrapperFunctionCall(arg, printfFunction) and
-  taintedWithPath(userValue, arg, sourceNode, sinkNode) and
-  isUserInput(userValue, cause)
-select arg, sourceNode, sinkNode,
+  source = sourceNode.getNode() and
+  sink = sinkNode.getNode() and
+  isSource(source, sourceType) and
+  printf.outermostWrapperFunctionCall([sink.asExpr(), sink.asIndirectExpr()], printfFunction) and
+  Flow::flowPath(sourceNode, sinkNode)
+select sink, sourceNode, sinkNode,
   "The value of this argument may come from $@ and is being used as a formatting argument to " +
-    printfFunction + ".", userValue, cause
+    printfFunction + ".", source, sourceType

cpp/ql/test/query-tests/Security/CWE/CWE-134/SAMATE/UncontrolledFormatString.expected

@@ -1,31 +1,16 @@
 edges
-| char_connect_socket_w32_vsnprintf_01_bad.c:94:46:94:69 | recv output argument | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data |
-| char_connect_socket_w32_vsnprintf_01_bad.c:94:46:94:69 | recv output argument | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data |
-| char_connect_socket_w32_vsnprintf_01_bad.c:94:55:94:68 | ... + ... | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data |
-| char_connect_socket_w32_vsnprintf_01_bad.c:94:55:94:68 | ... + ... | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data |
-| char_console_fprintf_01_bad.c:30:23:30:35 | ... + ... | char_console_fprintf_01_bad.c:49:21:49:24 | data |
-| char_console_fprintf_01_bad.c:30:23:30:35 | ... + ... | char_console_fprintf_01_bad.c:49:21:49:24 | data |
-| char_console_fprintf_01_bad.c:30:23:30:35 | fgets output argument | char_console_fprintf_01_bad.c:49:21:49:24 | data |
-| char_console_fprintf_01_bad.c:30:23:30:35 | fgets output argument | char_console_fprintf_01_bad.c:49:21:49:24 | data |
-| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | char_environment_fprintf_01_bad.c:36:21:36:24 | data |
-| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | char_environment_fprintf_01_bad.c:36:21:36:24 | data |
-| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | char_environment_fprintf_01_bad.c:36:21:36:24 | data |
-| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | char_environment_fprintf_01_bad.c:36:21:36:24 | data |
-subpaths
+| char_connect_socket_w32_vsnprintf_01_bad.c:94:46:94:69 | recv output argument | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data indirection |
+| char_console_fprintf_01_bad.c:30:23:30:35 | fgets output argument | char_console_fprintf_01_bad.c:49:21:49:24 | data indirection |
+| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv indirection | char_environment_fprintf_01_bad.c:36:21:36:24 | data indirection |
 nodes
 | char_connect_socket_w32_vsnprintf_01_bad.c:94:46:94:69 | recv output argument | semmle.label | recv output argument |
-| char_connect_socket_w32_vsnprintf_01_bad.c:94:55:94:68 | ... + ... | semmle.label | ... + ... |
-| char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data | semmle.label | data |
-| char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data | semmle.label | data |
-| char_console_fprintf_01_bad.c:30:23:30:35 | ... + ... | semmle.label | ... + ... |
+| char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data indirection | semmle.label | data indirection |
 | char_console_fprintf_01_bad.c:30:23:30:35 | fgets output argument | semmle.label | fgets output argument |
-| char_console_fprintf_01_bad.c:49:21:49:24 | data | semmle.label | data |
-| char_console_fprintf_01_bad.c:49:21:49:24 | data | semmle.label | data |
-| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | semmle.label | call to getenv |
-| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | semmle.label | call to getenv |
-| char_environment_fprintf_01_bad.c:36:21:36:24 | data | semmle.label | data |
-| char_environment_fprintf_01_bad.c:36:21:36:24 | data | semmle.label | data |
+| char_console_fprintf_01_bad.c:49:21:49:24 | data indirection | semmle.label | data indirection |
+| char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv indirection | semmle.label | call to getenv indirection |
+| char_environment_fprintf_01_bad.c:36:21:36:24 | data indirection | semmle.label | data indirection |
+subpaths
 #select
-| char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data | char_connect_socket_w32_vsnprintf_01_bad.c:94:55:94:68 | ... + ... | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data | The value of this argument may come from $@ and is being used as a formatting argument to badVaSink(data), which calls vsnprintf(format). | char_connect_socket_w32_vsnprintf_01_bad.c:94:55:94:68 | ... + ... | recv |
-| char_console_fprintf_01_bad.c:49:21:49:24 | data | char_console_fprintf_01_bad.c:30:23:30:35 | ... + ... | char_console_fprintf_01_bad.c:49:21:49:24 | data | The value of this argument may come from $@ and is being used as a formatting argument to fprintf(format). | char_console_fprintf_01_bad.c:30:23:30:35 | ... + ... | fgets |
-| char_environment_fprintf_01_bad.c:36:21:36:24 | data | char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | char_environment_fprintf_01_bad.c:36:21:36:24 | data | The value of this argument may come from $@ and is being used as a formatting argument to fprintf(format). | char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv | getenv |
+| char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data indirection | char_connect_socket_w32_vsnprintf_01_bad.c:94:46:94:69 | recv output argument | char_connect_socket_w32_vsnprintf_01_bad.c:125:15:125:18 | data indirection | The value of this argument may come from $@ and is being used as a formatting argument to badVaSink(data), which calls vsnprintf(format). | char_connect_socket_w32_vsnprintf_01_bad.c:94:46:94:69 | recv output argument | buffer read by recv |
+| char_console_fprintf_01_bad.c:49:21:49:24 | data indirection | char_console_fprintf_01_bad.c:30:23:30:35 | fgets output argument | char_console_fprintf_01_bad.c:49:21:49:24 | data indirection | The value of this argument may come from $@ and is being used as a formatting argument to fprintf(format). | char_console_fprintf_01_bad.c:30:23:30:35 | fgets output argument | string read by fgets |
+| char_environment_fprintf_01_bad.c:36:21:36:24 | data indirection | char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv indirection | char_environment_fprintf_01_bad.c:36:21:36:24 | data indirection | The value of this argument may come from $@ and is being used as a formatting argument to fprintf(format). | char_environment_fprintf_01_bad.c:27:30:27:35 | call to getenv indirection | an environment variable |

cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/argv/argvLocal.c

@@ -164,7 +164,7 @@ int main(int argc, char **argv) {
 	printf(i91);
 	printWrapper(i91);
 
-	// BAD: i10 value comes from argv
+	// BAD: i10 value comes from argv [NOT DETECTED]
 	int i10 = (int) argv[1];
 	printf((char *) i10);
 	printWrapper((char *) i10);