add bosted version of ShellCommandInjectionFromEnvironment

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/EndpointCharacteristics.qll

@@ -278,6 +278,28 @@ private class NosqlInjectionSinkCharacteristic extends EndpointCharacteristic {
   }
 }
 
+/**
+ * Endpoints identified as "ShellCommandInjectionFromEnvironmentSink" by the standard JavaScript libraries are
+ * ShellCommandInjectionFromEnvironment sinks with maximal confidence.
+ */
+private class ShellCommandInjectionFromEnvironmentSinkCharacteristic extends EndpointCharacteristic {
+  ShellCommandInjectionFromEnvironmentSinkCharacteristic() {
+    this = "ShellCommandInjectionFromEnvironmentSink"
+  }
+
+  override predicate appliesToEndpoint(DataFlow::Node n) {
+    n instanceof ShellCommandInjectionFromEnvironment::Sink
+  }
+
+  override predicate hasImplications(
+    EndpointType endpointClass, boolean isPositiveIndicator, float confidence
+  ) {
+    endpointClass instanceof ShellCommandInjectionFromEnvironmentSinkType and
+    isPositiveIndicator = true and
+    confidence = maximalConfidence()
+  }
+}
+
 /*
  * Characteristics that are indicative of not being a sink of any type, and have historically been used to select
  * negative samples for training.

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/EndpointTypes.qll

@@ -10,7 +10,8 @@ newtype TEndpointType =
   TXssSinkType() or
   TNosqlInjectionSinkType() or
   TSqlInjectionSinkType() or
-  TTaintedPathSinkType()
+  TTaintedPathSinkType() or
+  TShellCommandInjectionFromEnvironmentSinkType()
 
 /** A class that can be predicted by endpoint scoring models. */
 abstract class EndpointType extends TEndpointType {
@@ -60,3 +61,11 @@ class TaintedPathSinkType extends EndpointType, TTaintedPathSinkType {
 
   override int getEncoding() { result = 4 }
 }
+
+/** The `ShellCommandInjectionFromEnvironmentSink` class that can be predicted by endpoint scoring models. */
+class ShellCommandInjectionFromEnvironmentSinkType extends EndpointType,
+  TShellCommandInjectionFromEnvironmentSinkType {
+  override string getDescription() { result = "ShellCommandInjectionFromEnvironmentSink" }
+
+  override int getEncoding() { result = 5 }
+}

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/ShellCommandInjectionFromEnvironmentATM.qll

@@ -0,0 +1,30 @@
+/**
+ * For internal use only.
+ *
+ * A taint-tracking configuration for reasoning about command-injection
+ * vulnerabilities.
+ * Defines shared code used by the ShellCommandInjectionFromEnvironment boosted query.
+ */
+
+private import semmle.javascript.heuristics.SyntacticHeuristics
+private import semmle.javascript.security.dataflow.ShellCommandInjectionFromEnvironmentCustomizations::ShellCommandInjectionFromEnvironment as ShellCommandInjectionFromEnvironment
+import AdaptiveThreatModeling
+
+class ShellCommandInjectionFromEnvironmentAtmConfig extends AtmConfig {
+  ShellCommandInjectionFromEnvironmentAtmConfig() {
+    this = "ShellCommandInjectionFromEnvironmentAtmConfig"
+  }
+
+  override predicate isKnownSource(DataFlow::Node source) {
+    source instanceof ShellCommandInjectionFromEnvironment::Source
+  }
+
+  override EndpointType getASinkEndpointType() {
+    result instanceof ShellCommandInjectionFromEnvironmentSinkType
+  }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    super.isSanitizer(node) or
+    node instanceof ShellCommandInjectionFromEnvironment::Sanitizer
+  }
+}

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/DebugResultInclusion.ql

@@ -17,6 +17,7 @@ private import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjecti
 private import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm
 private import experimental.adaptivethreatmodeling.XssATM as XssAtm
 private import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomAtm
+private import experimental.adaptivethreatmodeling.ShellCommandInjectionFromEnvironmentATM as ShellCommandInjectionFromEnvironmentAtm
 
 string getAReasonSinkExcluded(DataFlow::Node sinkCandidate, Query query) {
   query instanceof NosqlInjectionQuery and
@@ -33,6 +34,11 @@ string getAReasonSinkExcluded(DataFlow::Node sinkCandidate, Query query) {
   or
   query instanceof XssThroughDomQuery and
   result = any(XssThroughDomAtm::XssThroughDomAtmConfig cfg).getAReasonSinkExcluded(sinkCandidate)
+  or
+  query instanceof ShellCommandInjectionFromEnvironmentQuery and
+  result =
+    any(ShellCommandInjectionFromEnvironmentAtm::ShellCommandInjectionFromEnvironmentAtmConfig cfg)
+        .getAReasonSinkExcluded(sinkCandidate)
 }
 
 pragma[inline]

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractEndpointDataTraining.qll

@@ -15,6 +15,7 @@ private import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjecti
 private import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm
 private import experimental.adaptivethreatmodeling.XssATM as XssAtm
 private import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomAtm
+private import experimental.adaptivethreatmodeling.ShellCommandInjectionFromEnvironmentATM as ShellCommandInjectionFromEnvironmentAtm
 
 /**
  * Gets the set of featureName-featureValue pairs for each endpoint in the training set.
@@ -217,6 +218,10 @@ DataFlow::Configuration getDataFlowCfg(Query query) {
   query instanceof XssQuery and result instanceof XssAtm::DomBasedXssAtmConfig
   or
   query instanceof XssThroughDomQuery and result instanceof XssThroughDomAtm::XssThroughDomAtmConfig
+  or
+  query instanceof ShellCommandInjectionFromEnvironmentQuery and
+  result instanceof
+    ShellCommandInjectionFromEnvironmentAtm::ShellCommandInjectionFromEnvironmentAtmConfig
 }
 
 // TODO: Delete this once we are no longer surfacing `hasFlowFromSource`.

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractEndpointMapping.ql

@@ -9,6 +9,7 @@ import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInjectionAt
 import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm
 import experimental.adaptivethreatmodeling.XssATM as XssAtm
 import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomAtm
+import experimental.adaptivethreatmodeling.ShellCommandInjectionFromEnvironmentATM as ShellCommandInjectionFromEnvironmentAtm
 import experimental.adaptivethreatmodeling.AdaptiveThreatModeling
 
 from string queryName, AtmConfig c, EndpointType e
@@ -26,6 +27,10 @@ where
     queryName = "Xss" and c instanceof XssAtm::DomBasedXssAtmConfig
     or
     queryName = "XssThroughDom" and c instanceof XssThroughDomAtm::XssThroughDomAtmConfig
+    or
+    queryName = "ShellCommandInjectionFromEnvironment" and
+    c instanceof
+      ShellCommandInjectionFromEnvironmentAtm::ShellCommandInjectionFromEnvironmentAtmConfig
   ) and
   e = c.getASinkEndpointType()
 select queryName, e.getEncoding() as label

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/Queries.qll

@@ -9,7 +9,8 @@ newtype TQuery =
   TSqlInjectionQuery() or
   TTaintedPathQuery() or
   TXssQuery() or
-  TXssThroughDomQuery()
+  TXssThroughDomQuery() or
+  TShellCommandInjectionFromEnvironmentQuery()
 
 abstract class Query extends TQuery {
   abstract string getName();
@@ -36,3 +37,8 @@ class XssQuery extends Query, TXssQuery {
 class XssThroughDomQuery extends Query, TXssThroughDomQuery {
   override string getName() { result = "XssThroughDom" }
 }
+
+class ShellCommandInjectionFromEnvironmentQuery extends Query,
+  TShellCommandInjectionFromEnvironmentQuery {
+  override string getName() { result = "ShellCommandInjectionFromEnvironment" }
+}

javascript/ql/experimental/adaptivethreatmodeling/src/ShellCommandInjectionFromEnvironmentATM.ql

@@ -0,0 +1,29 @@
+/**
+ * For internal use only.
+ *
+ * @name Shell command built from environment values
+ * @description Building a shell command string with values from the enclosing
+ *              environment may cause subtle bugs or vulnerabilities.
+ * @kind path-problem
+ * @scored
+ * @problem.severity warning
+ * @security-severity 6.3
+ * @precision high
+ * @id js/ml-powered/shell-command-injection-from-environment
+ * @tags experimental security
+ *       correctness
+ *       security
+ *       external/cwe/cwe-078
+ *       external/cwe/cwe-088
+ */
+
+import javascript
+import experimental.adaptivethreatmodeling.ShellCommandInjectionFromEnvironmentATM
+import ATM::ResultsInfo
+import DataFlow::PathGraph
+
+from AtmConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink, float score
+where cfg.hasBoostedFlowPath(source, sink, score)
+select sink.getNode(), source, sink,
+  "(Experimental) This shell command depends on $@. Identified using machine learning.",
+  source.getNode(), "an uncontrolled value", score

javascript/ql/experimental/adaptivethreatmodeling/test/endpoint_large_scale/EndpointFeatures.ql

@@ -12,6 +12,7 @@ import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionAtm
 import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm
 import experimental.adaptivethreatmodeling.XssATM as XssAtm
 import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomAtm
+import experimental.adaptivethreatmodeling.ShellCommandInjectionFromEnvironmentATM as ShellCommandInjectionFromEnvironmentAtm
 import experimental.adaptivethreatmodeling.EndpointFeatures as EndpointFeatures
 import extraction.NoFeaturizationRestrictionsConfig
 private import experimental.adaptivethreatmodeling.EndpointCharacteristics as EndpointCharacteristics
@@ -23,6 +24,10 @@ query predicate tokenFeatures(DataFlow::Node endpoint, string featureName, strin
     not exists(any(TaintedPathAtm::TaintedPathAtmConfig cfg).getAReasonSinkExcluded(endpoint)) or
     not exists(any(XssAtm::DomBasedXssAtmConfig cfg).getAReasonSinkExcluded(endpoint)) or
     not exists(any(XssThroughDomAtm::XssThroughDomAtmConfig cfg).getAReasonSinkExcluded(endpoint)) or
+    not exists(
+      any(ShellCommandInjectionFromEnvironmentAtm::ShellCommandInjectionFromEnvironmentAtmConfig cfg)
+          .getAReasonSinkExcluded(endpoint)
+    ) or
     any(EndpointCharacteristics::IsArgumentToModeledFunctionCharacteristic characteristic)
         .appliesToEndpoint(endpoint)
   ) and

javascript/ql/experimental/adaptivethreatmodeling/test/endpoint_large_scale/ExtractEndpointDataInference.ql

@@ -19,6 +19,7 @@ private import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjecti
 private import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm
 private import experimental.adaptivethreatmodeling.XssATM as XssAtm
 private import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomAtm
+private import experimental.adaptivethreatmodeling.ShellCommandInjectionFromEnvironmentATM as ShellCommandInjectionFromEnvironmentAtm
 
 query predicate isSinkCandidateForQuery(
   AtmConfig::AtmConfig queryConfig, JS::DataFlow::PathNode sink

javascript/ql/experimental/adaptivethreatmodeling/test/endpoint_large_scale/FilteredTruePositives.ql

@@ -16,11 +16,13 @@ import semmle.javascript.security.dataflow.NosqlInjectionCustomizations
 import semmle.javascript.security.dataflow.SqlInjectionCustomizations
 import semmle.javascript.security.dataflow.TaintedPathCustomizations
 import semmle.javascript.security.dataflow.DomBasedXssCustomizations
+import semmle.javascript.security.dataflow.ShellCommandInjectionFromEnvironmentCustomizations
 import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInjectionAtm
 import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionAtm
 import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm
 import experimental.adaptivethreatmodeling.XssATM as XssAtm
 import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomAtm
+import experimental.adaptivethreatmodeling.ShellCommandInjectionFromEnvironmentATM as ShellCommandInjectionFromEnvironmentAtm
 
 query predicate nosqlFilteredTruePositives(DataFlow::Node endpoint, string reason) {
   endpoint instanceof NosqlInjection::Sink and
@@ -51,3 +53,13 @@ query predicate xssThroughDomFilteredTruePositives(DataFlow::Node endpoint, stri
   reason = any(XssThroughDomAtm::XssThroughDomAtmConfig cfg).getAReasonSinkExcluded(endpoint) and
   reason != "argument to modeled function"
 }
+
+query predicate shellCommandInjectionFromEnvironmentAtmFilteredTruePositives(
+  DataFlow::Node endpoint, string reason
+) {
+  endpoint instanceof ShellCommandInjectionFromEnvironment::Sink and
+  reason =
+    any(ShellCommandInjectionFromEnvironmentAtm::ShellCommandInjectionFromEnvironmentAtmConfig cfg)
+        .getAReasonSinkExcluded(endpoint) and
+  reason != "argument to modeled function"
+}