Merge branch 'main' into alexdenisov/macros

Author: AlexDenisov

CODEOWNERS

@@ -8,7 +8,7 @@
 /swift/ @github/codeql-swift
 /misc/codegen/ @github/codeql-swift
 /java/kotlin-extractor/ @github/codeql-kotlin
-/java/ql/test/kotlin/ @github/codeql-kotlin
+/java/ql/test-kotlin1/ @github/codeql-kotlin
 /java/ql/test-kotlin2/ @github/codeql-kotlin
 
 # ML-powered queries

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

@@ -59,6 +59,9 @@ private module SourceVariables {
       then result = base.getType()
       else result = getTypeImpl(base.getType(), ind - 1)
     }
+
+    /** Gets the location of this variable. */
+    Location getLocation() { result = this.getBaseVariable().getLocation() }
   }
 }
 
@@ -869,7 +872,7 @@ private predicate sourceVariableIsGlobal(
   )
 }
 
-private module SsaInput implements SsaImplCommon::InputSig {
+private module SsaInput implements SsaImplCommon::InputSig<Location> {
   import InputSigCommon
   import SourceVariables
 
@@ -1092,7 +1095,7 @@ class Def extends DefOrUse {
   predicate isCertain() { defOrUse.isCertain() }
 }
 
-private module SsaImpl = SsaImplCommon::Make<SsaInput>;
+private module SsaImpl = SsaImplCommon::Make<Location, SsaInput>;
 
 class PhiNode extends SsaImpl::DefinitionExt {
   PhiNode() {

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternalsCommon.qll

@@ -377,6 +377,9 @@ abstract private class AbstractBaseSourceVariable extends TBaseSourceVariable {
   /** Gets a textual representation of this element. */
   abstract string toString();
 
+  /** Gets the location of this variable. */
+  abstract Location getLocation();
+
   /** Gets the type of this base source variable. */
   final DataFlowType getType() { this.getLanguageType().hasUnspecifiedType(result, _) }
 
@@ -395,6 +398,8 @@ class BaseIRVariable extends AbstractBaseSourceVariable, TBaseIRVariable {
 
   override string toString() { result = var.toString() }
 
+  override Location getLocation() { result = var.getLocation() }
+
   override CppType getLanguageType() { result = var.getLanguageType() }
 }
 
@@ -407,6 +412,8 @@ class BaseCallVariable extends AbstractBaseSourceVariable, TBaseCallVariable {
 
   override string toString() { result = call.toString() }
 
+  override Location getLocation() { result = call.getLocation() }
+
   override CppType getLanguageType() { result = getResultLanguageType(call) }
 }
 

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ssa0/SsaInternals.qll

@@ -229,7 +229,7 @@ private class FinalParameterUse extends UseImpl, TFinalParameterUse {
   override predicate isCertain() { any() }
 }
 
-private module SsaInput implements SsaImplCommon::InputSig {
+private module SsaInput implements SsaImplCommon::InputSig<Location> {
   import InputSigCommon
   import SourceVariables
 
@@ -335,7 +335,7 @@ class Def extends DefOrUse {
   predicate isIteratorDef() { defOrUse instanceof IteratorDef }
 }
 
-private module SsaImpl = SsaImplCommon::Make<SsaInput>;
+private module SsaImpl = SsaImplCommon::Make<Location, SsaInput>;
 
 class PhiNode extends SsaImpl::DefinitionExt {
   PhiNode() {

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);