Merge branch 'main' into topPack

javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll

@@ -738,7 +738,10 @@ module TaintTracking {
   pragma[nomagic]
   private DataFlow::MethodCallNode execMethodCall() {
     result.getMethodName() = "exec" and
-    result.getReceiver().analyze().getAType() = TTRegExp()
+    exists(DataFlow::AnalyzedNode analyzed |
+      pragma[only_bind_into](analyzed) = result.getReceiver().analyze() and
+      analyzed.getAType() = TTRegExp()
+    )
   }
 
   /**
@@ -759,7 +762,10 @@ module TaintTracking {
   pragma[nomagic]
   private DataFlow::MethodCallNode matchMethodCall() {
     result.getMethodName() = "match" and
-    result.getArgument(0).analyze().getAType() = TTRegExp()
+    exists(DataFlow::AnalyzedNode analyzed |
+      pragma[only_bind_into](analyzed) = result.getArgument(0).analyze() and
+      analyzed.getAType() = TTRegExp()
+    )
   }
 
   /**

javascript/ql/src/semmle/javascript/frameworks/AngularJS/ServiceDefinitions.qll

@@ -297,34 +297,29 @@ abstract private class CustomSpecialServiceDefinition extends CustomServiceDefin
 /**
  * Holds if `mce` defines a service of type `moduleMethodName` with name `serviceName` using the `factoryFunction` as the factory function.
  */
+bindingset[moduleMethodName]
 private predicate isCustomServiceDefinitionOnModule(
   DataFlow::CallNode mce, string moduleMethodName, string serviceName,
-  DataFlow::SourceNode factoryFunction
+  DataFlow::Node factoryArgument
 ) {
   mce = moduleRef(_).getAMethodCall(moduleMethodName) and
-  (
-    moduleMethodName = "controller" or
-    moduleMethodName = "filter" or
-    moduleMethodName = "directive" or
-    moduleMethodName = "component" or
-    moduleMethodName = "animation"
-  ) and
   mce.getArgument(0).asExpr().mayHaveStringValue(serviceName) and
-  factoryFunction.flowsTo(mce.getArgument(1))
+  factoryArgument = mce.getArgument(1)
 }
 
+pragma[inline]
 private predicate isCustomServiceDefinitionOnProvider(
   DataFlow::CallNode mce, string providerName, string providerMethodName, string serviceName,
-  DataFlow::SourceNode factoryFunction
+  DataFlow::Node factoryArgument
 ) {
   mce = builtinServiceRef(providerName).getAMethodCall(providerMethodName) and
   (
     mce.getNumArgument() = 1 and
-    factoryFunction.flowsTo(mce.getOptionArgument(0, serviceName))
+    factoryArgument = mce.getOptionArgument(0, serviceName)
     or
     mce.getNumArgument() = 2 and
     mce.getArgument(0).asExpr().mayHaveStringValue(serviceName) and
-    factoryFunction.flowsTo(mce.getArgument(1))
+    factoryArgument = mce.getArgument(1)
   )
 }
 
@@ -333,7 +328,7 @@ private predicate isCustomServiceDefinitionOnProvider(
  */
 class ControllerDefinition extends CustomSpecialServiceDefinition {
   string name;
-  DataFlow::SourceNode factoryFunction;
+  DataFlow::Node factoryFunction;
 
   ControllerDefinition() {
     isCustomServiceDefinitionOnModule(this, "controller", name, factoryFunction) or
@@ -343,9 +338,9 @@ class ControllerDefinition extends CustomSpecialServiceDefinition {
 
   override string getName() { result = name }
 
-  override DataFlow::SourceNode getAService() { result = factoryFunction }
+  override DataFlow::SourceNode getAService() { result = factoryFunction.getALocalSource() }
 
-  override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction }
+  override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction.getALocalSource() }
 }
 
 /**
@@ -353,7 +348,7 @@ class ControllerDefinition extends CustomSpecialServiceDefinition {
  */
 class FilterDefinition extends CustomSpecialServiceDefinition {
   string name;
-  DataFlow::SourceNode factoryFunction;
+  DataFlow::Node factoryFunction;
 
   FilterDefinition() {
     isCustomServiceDefinitionOnModule(this, "filter", name, factoryFunction) or
@@ -364,12 +359,12 @@ class FilterDefinition extends CustomSpecialServiceDefinition {
 
   override DataFlow::SourceNode getAService() {
     exists(InjectableFunction f |
-      f = factoryFunction and
+      f = factoryFunction.getALocalSource() and
       result.flowsToExpr(f.asFunction().getAReturnedExpr())
     )
   }
 
-  override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction }
+  override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction.getALocalSource() }
 }
 
 /**
@@ -377,7 +372,7 @@ class FilterDefinition extends CustomSpecialServiceDefinition {
  */
 class DirectiveDefinition extends CustomSpecialServiceDefinition {
   string name;
-  DataFlow::SourceNode factoryFunction;
+  DataFlow::Node factoryFunction;
 
   DirectiveDefinition() {
     isCustomServiceDefinitionOnModule(this, "directive", name, factoryFunction) or
@@ -393,7 +388,7 @@ class DirectiveDefinition extends CustomSpecialServiceDefinition {
     )
   }
 
-  override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction }
+  override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction.getALocalSource() }
 }
 
 private class CustomDirectiveControllerDependencyInjection extends DependencyInjection {
@@ -416,7 +411,7 @@ private class CustomDirectiveControllerDependencyInjection extends DependencyInj
  */
 class ComponentDefinition extends CustomSpecialServiceDefinition {
   string name;
-  DataFlow::SourceNode config;
+  DataFlow::Node config;
 
   ComponentDefinition() {
     isCustomServiceDefinitionOnModule(this, "component", name, config) or
@@ -435,7 +430,7 @@ class ComponentDefinition extends CustomSpecialServiceDefinition {
   override DataFlow::SourceNode getAFactoryFunction() { none() }
 
   /** Gets the configuration object for the defined component. */
-  DataFlow::SourceNode getConfig() { result = config }
+  DataFlow::SourceNode getConfig() { result = config.getALocalSource() }
 }
 
 /**
@@ -443,7 +438,7 @@ class ComponentDefinition extends CustomSpecialServiceDefinition {
  */
 class AnimationDefinition extends CustomSpecialServiceDefinition {
   string name;
-  DataFlow::SourceNode factoryFunction;
+  DataFlow::Node factoryFunction;
 
   AnimationDefinition() {
     isCustomServiceDefinitionOnModule(this, "animation", name, factoryFunction) or
@@ -454,12 +449,12 @@ class AnimationDefinition extends CustomSpecialServiceDefinition {
 
   override DataFlow::SourceNode getAService() {
     exists(InjectableFunction f |
-      f = factoryFunction and
+      f = factoryFunction.getALocalSource() and
       result.flowsToExpr(f.asFunction().getAReturnedExpr())
     )
   }
 
-  override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction }
+  override DataFlow::SourceNode getAFactoryFunction() { result = factoryFunction.getALocalSource() }
 }
 
 /**
@@ -500,7 +495,7 @@ private class LinkFunctionWithScopeInjection extends ServiceRequest {
 class InjectableFunctionServiceRequest extends ServiceRequest {
   InjectableFunction injectedFunction;
 
-  InjectableFunctionServiceRequest() { DataFlow::valueNode(this) = injectedFunction }
+  InjectableFunctionServiceRequest() { injectedFunction.getAstNode() = this }
 
   /**
    * Gets the function of this request.
@@ -589,7 +584,7 @@ class ServiceRecipeDefinition extends RecipeDefinition {
 
     exists(InjectableFunction f |
       f = getAFactoryFunction() and
-      result = DataFlow::valueNode(f.asFunction())
+      result.getAstNode() = f.asFunction()
     )
   }
 }

javascript/ql/src/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll

@@ -143,6 +143,53 @@ module UnsafeShellCommandConstruction {
     override DataFlow::Node getAlertLocation() { result = this }
   }
 
+  /**
+   * Gets a node that ends up in an array that is ultimately executed as a shell script by `sys`.
+   */
+  private DataFlow::SourceNode endsInShellExecutedArray(
+    DataFlow::TypeBackTracker t, SystemCommandExecution sys
+  ) {
+    t.start() and
+    result = sys.getArgumentList().getALocalSource() and
+    // the array gets joined to a string when `shell` is set to true.
+    sys.getOptionsArg()
+        .getALocalSource()
+        .getAPropertyWrite("shell")
+        .getRhs()
+        .asExpr()
+        .(BooleanLiteral)
+        .getValue() = "true"
+    or
+    exists(DataFlow::TypeBackTracker t2 |
+      result = endsInShellExecutedArray(t2, sys).backtrack(t2, t)
+    )
+  }
+
+  /**
+   * An argument to a command invocation where the `shell` option is set to true.
+   */
+  class ShellTrueCommandExecutionSink extends Sink {
+    SystemCommandExecution sys;
+
+    ShellTrueCommandExecutionSink() {
+      // `shell` is set to true. That means string-concatenation happens behind the scenes.
+      // We just assume that a `shell` option in any library means the same thing as it does in NodeJS.
+      exists(DataFlow::SourceNode arr |
+        arr = endsInShellExecutedArray(DataFlow::TypeBackTracker::end(), sys)
+      |
+        this = arr.(DataFlow::ArrayCreationNode).getAnElement()
+        or
+        this = arr.getAMethodCall(["push", "unshift"]).getAnArgument()
+      )
+    }
+
+    override string getSinkType() { result = "Shell argument" }
+
+    override SystemCommandExecution getCommandExecution() { result = sys }
+
+    override DataFlow::Node getAlertLocation() { result = this }
+  }
+
   /**
    * A sanitizer like: "'"+name.replace(/'/g,"'\\''")+"'"
    * Which sanitizes on Unix.