Merge pull request #368 from microsoft/copilot/ps-deserialization-precision-20260615

Improve PowerShell deserialization precision

Author: chanel-y

powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll

@@ -743,6 +743,8 @@ class FlowSummaryNode extends NodeImpl, TFlowSummaryNode {
 
   override EmptyLocation getLocationImpl() { any() }
 
+  override predicate nodeIsHidden() { any() }
+
   override string toStringImpl() { result = this.getSummaryNode().toString() }
 }
 

powershell/ql/lib/semmle/code/powershell/frameworks/System.IO.model.yml

@@ -37,6 +37,8 @@ extensions:
       extensible: summaryModel
     data:
       - ["system.io.path!", "Method[getfullpath]", "Argument[0]", "ReturnValue", "taint"]
+      - ["system.io.memorystream!", "Method[new]", "Argument[0]", "ReturnValue", "taint"]
+      - ["system.io.stringreader!", "Method[new]", "Argument[0]", "ReturnValue", "taint"]
       - ["system.io.file!", "Method[readallbytes]", "Argument[0]", "ReturnValue", "taint"]
       - ["system.io.file!", "Method[readallbytesasync]", "Argument[0]", "ReturnValue", "taint"]
       - ["system.io.file!", "Method[appendtext]", "Argument[0]", "ReturnValue", "taint"]
@@ -45,4 +47,5 @@ extensions:
       - ["system.io.file!", "Method[readalllinesasync]", "Argument[0]", "ReturnValue", "taint"]
       - ["system.io.file!", "Method[readalltext]", "Argument[0]", "ReturnValue", "taint"]
       - ["system.io.file!", "Method[readalltextasync]", "Argument[0]", "ReturnValue", "taint"]
-      - ["system.io.fileinfo", "Method[createtext]", "Argument[0]", "ReturnValue", "taint"]
+      - ["system.io.fileinfo", "Method[createtext]", "Argument[0]", "ReturnValue", "taint"]
+

powershell/ql/lib/semmle/code/powershell/frameworks/System.model.yml

@@ -10,3 +10,9 @@ extensions:
       - ["system.environment!", "Method[getcommandlineargs].ReturnValue", "command-line"]
       - ["system.environment!", "Method[getenvironmentvariable].ReturnValue", "environment"]
       - ["system.environment!", "Method[getenvironmentvariables].ReturnValue", "environment"]
+
+  - addsTo:
+      pack: microsoft/powershell-all
+      extensible: summaryModel
+    data:
+      - ["system.convert!", "Method[frombase64string]", "Argument[0]", "ReturnValue", "taint"]

powershell/ql/lib/semmle/code/powershell/security/UnsafeDeserializationCustomizations.qll

@@ -27,11 +27,6 @@ module UnsafeDeserialization {
     abstract string getSinkType();
   }
 
-  /**
-   * A sanitizer for Unsafe Deserialization vulnerabilities.
-   */
-  abstract class Sanitizer extends DataFlow::Node { }
-
   /** A source of user input, considered as a flow source for unsafe deserialization. */
   class FlowSourceAsSource extends Source instanceof SourceNode {
     override string getSourceType() { result = SourceNode.super.getSourceType() }
@@ -42,14 +37,16 @@ module UnsafeDeserialization {
    * (lowercase) matches `fullTypeName`. Handles both `New-Object TypeName` and
    * `[TypeName]::new()` patterns.
    */
-  private predicate objectCreationMatchesType(
-    DataFlow::ObjectCreationNode ocn, string fullTypeName
-  ) {
+  private predicate objectCreationMatchesType(DataFlow::ObjectCreationNode ocn, string fullTypeName) {
     // New-Object TypeName: getLowerCaseConstructedTypeName() returns the full qualified name
     ocn.getLowerCaseConstructedTypeName() = fullTypeName
     or
     // [TypeName]::new(): access the qualifier TypeNameExpr for the full qualified name
-    ocn.getExprNode().getExpr().(ConstructorCall).getQualifier().(TypeNameExpr)
+    ocn.getExprNode()
+        .getExpr()
+        .(ConstructorCall)
+        .getQualifier()
+        .(TypeNameExpr)
         .getPossiblyQualifiedName() = fullTypeName
   }
 
@@ -106,20 +103,28 @@ module UnsafeDeserialization {
   }
 
   /**
-   * An argument to a BinaryFormatter deserialization method call, including
-   * Deserialize, UnsafeDeserialize, and UnsafeDeserializeMethodResponse.
+   * A BinaryFormatter deserialization method call, including Deserialize, UnsafeDeserialize,
+   * and UnsafeDeserializeMethodResponse.
    */
-  class BinaryFormatterDeserializeSink extends Sink {
-    BinaryFormatterDeserializeSink() {
-      exists(DataFlow::ObjectCreationNode ocn, DataFlow::CallNode cn |
-        cn.getQualifier().getALocalSource() = ocn and
+  class BinaryFormatterDeserializeCall extends DataFlow::CallNode {
+    BinaryFormatterDeserializeCall() {
+      exists(DataFlow::ObjectCreationNode ocn |
+        this.getQualifier().getALocalSource() = ocn and
         objectCreationMatchesType(ocn,
           "system.runtime.serialization.formatters.binary.binaryformatter") and
-        cn.getLowerCaseName() =
-          ["deserialize", "unsafedeserialize", "unsafedeserializemethodresponse"] and
-        cn.getAnArgument() = this
+        this.getLowerCaseName() =
+          ["deserialize", "unsafedeserialize", "unsafedeserializemethodresponse"]
       )
     }
+  }
+
+  /**
+   * An input argument to a BinaryFormatter deserialization method call.
+   */
+  class BinaryFormatterDeserializeSink extends Sink {
+    BinaryFormatterDeserializeSink() {
+      exists(BinaryFormatterDeserializeCall cn | cn.getAnArgument() = this)
+    }
 
     override string getSinkType() { result = "call to BinaryFormatter.Deserialize" }
   }
@@ -167,9 +172,7 @@ module UnsafeDeserialization {
       )
     }
 
-    override string getSinkType() {
-      result = "call to [" + typeName + "]::" + methodName
-    }
+    override string getSinkType() { result = "call to [" + typeName + "]::" + methodName }
   }
 
   /**

powershell/ql/lib/semmle/code/powershell/security/UnsafeDeserializationQuery.qll

@@ -17,12 +17,14 @@ module Config implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source instanceof Source }
 
   predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
-  predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo){
-    exists(InvokeMemberExpr ime | 
-        nodeTo.asExpr().getExpr() = ime and 
-        nodeFrom.asExpr().getExpr() = ime.getAnArgument()
+
+  predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    exists(InvokeMemberExpr ime |
+      ime.getLowerCaseName() = "getbytes" and
+      nodeTo.asExpr().getExpr() = ime and
+      nodeFrom.asExpr().getExpr() = ime.getAnArgument()
     )
   }
 }
 
-module UnsafeDeserializationFlow = TaintTracking::Global<Config>; 
+module UnsafeDeserializationFlow = TaintTracking::Global<Config>;

powershell/ql/src/queries/security/cwe-502/BinaryFormatterDeserialization.qhelp

@@ -4,19 +4,28 @@
 <qhelp>
 <overview>
 
-<p>Using <code>BinaryFormatter</code> to deserialize an object from untrusted input may result in security problems, such
-as denial of service or remote code execution.</p>
+<p><code>BinaryFormatter</code> is unsafe for deserialization and should not be used, even when
+the data is produced locally. This query is an API-use warning: it reports calls to
+<code>BinaryFormatter</code> deserialization methods without proving that the serialized data is
+attacker-controlled.</p>
+
+<p>When attacker-controlled data is known to flow into a deserializer, the
+<code>powershell/microsoft/public/unsafe-deserialization</code> query reports that higher-confidence
+data-flow result.</p>
 
 </overview>
 <recommendation>
 
-<p>Avoid using <code>BinaryFormatter</code>.</p>
+<p>Avoid using <code>BinaryFormatter</code>. Prefer serializers that do not permit arbitrary type
+instantiation, and validate or authenticate any serialized data before deserializing it.</p>
 
 </recommendation>
 <example>
 
 <p>In this example, a string is deserialized using a
-<code>BinaryFormatter</code>. <code>BinaryFormatter</code> is an easily exploited deserializer.</p>
+<code>BinaryFormatter</code>. <code>BinaryFormatter</code> is an easily exploited deserializer. If the
+string is attacker-controlled, this is also reported by the taint-tracking unsafe
+deserialization query.</p>
 
 <sample src="examples/BinaryFormatterDeserialization.ps1" />
 

powershell/ql/src/queries/security/cwe-502/BinaryFormatterDeserialization.ql

@@ -1,9 +1,9 @@
 /**
- * @name Use of Binary Formatter deserialization
- * @description Use of Binary Formatter is unsafe
+ * @name Use of BinaryFormatter deserialization API
+ * @description BinaryFormatter deserialization is unsafe even when the input is trusted.
  * @kind problem
- * @problem.severity error
- * @security-severity 8.8
+ * @problem.severity warning
+ * @security-severity 7.5
  * @precision high
  * @id powershell/microsoft/public/binary-formatter-deserialization
  * @tags correctness
@@ -14,5 +14,6 @@
 import powershell
 import semmle.code.powershell.security.UnsafeDeserializationCustomizations::UnsafeDeserialization
 
-from BinaryFormatterDeserializeSink sink
-select sink, "Call to BinaryFormatter.Deserialize"
+from BinaryFormatterDeserializeCall call
+select call,
+  "This call uses BinaryFormatter deserialization. BinaryFormatter is unsafe; if this data can be attacker-controlled, the unsafe-deserialization query reports the data flow."

powershell/ql/src/queries/security/cwe-502/UnsafeDeserialization.qhelp

@@ -4,13 +4,21 @@
 <qhelp>
 <overview>
 
-<p>Deserializing an object from untrusted input may result in security problems, such
-as denial of service or remote code execution.</p>
+<p>Deserializing attacker-controlled data with an unsafe deserializer may result in
+security problems, such as denial of service or remote code execution. This query tracks
+untrusted data flow into unsafe deserialization APIs.</p>
+
+<p>Local, self-produced serialized data without any untrusted source is not reported by
+this query. The separate <code>powershell/microsoft/public/binary-formatter-deserialization</code>
+query reports <code>BinaryFormatter</code> API use even when no attacker-controlled data flow is
+known.</p>
 
 </overview>
 <recommendation>
 
-<p>Avoid using an unsafe deserialization framework.</p>
+<p>Avoid using unsafe deserialization frameworks for attacker-controlled data. Prefer safe
+serializers, and validate, authenticate, or otherwise constrain serialized data before
+deserializing it.</p>
 
 </recommendation>
 <example>