Swift: Models and tests for numeric conversions

swift/ql/lib/change-notes/2023-08-10-numeric-models.md

@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+
+* Added taint models for `Numeric` conversions.

swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll

@@ -213,6 +213,11 @@ private module Cached {
     //   retaining this case increases robustness of flow).
     nodeFrom.asExpr() = nodeTo.asExpr().(ForceValueExpr).getSubExpr()
     or
+    // read of an optional .some member via `let x: T = y: T?` pattern matching
+    // note: similar to `ForceValueExpr` this is ideally a content `readStep` but
+    //   in practice we sometimes have taint on the optional itself.
+    nodeTo.asPattern() = nodeFrom.asPattern().(OptionalSomePattern).getSubPattern()
+    or
     // flow through `?` and `?.`
     nodeFrom.asExpr() = nodeTo.asExpr().(BindOptionalExpr).getSubExpr()
     or

swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Array.qll

@@ -19,6 +19,10 @@ private class ArraySummaries extends SummaryModelCsv {
   override predicate row(string row) {
     row =
       [
+        ";Array;true;init(_:);;;Argument[0];ReturnValue.CollectionElement;value",
+        ";Array;true;init(_:);;;Argument[0].CollectionElement;ReturnValue.CollectionElement;value",
+        ";Array;true;init(repeating:count:);;;Argument[0];ReturnValue.CollectionElement;value",
+        ";Array;true;init(arrayLiteral:);;;Argument[0].CollectionElement;ReturnValue.CollectionElement;value",
         ";Array;true;insert(_:at:);;;Argument[0];Argument[-1].CollectionElement;value",
         ";Array;true;insert(_:at:);;;Argument[1];Argument[-1];taint",
         ";Array;true;withUnsafeBufferPointer(_:);;;Argument[-1];Argument[0].Parameter[0].CollectionElement;taint",

swift/ql/lib/codeql/swift/frameworks/StandardLibrary/CInterop.qll

@@ -7,6 +7,6 @@ private import codeql.swift.dataflow.ExternalFlow
 
 private class CInteropSummaries extends SummaryModelCsv {
   override predicate row(string row) {
-    row = ";;false;getVaList(_:);;;Argument[0].ArrayElement;ReturnValue;value"
+    row = ";;false;getVaList(_:);;;Argument[0].CollectionElement;ReturnValue;value"
   }
 }

swift/ql/lib/codeql/swift/frameworks/StandardLibrary/Numeric.qll

@@ -0,0 +1,53 @@
+/**
+ * Provides models for `Numeric` and related Swift classes (such as `Int` and `Float`).
+ */
+
+import swift
+private import codeql.swift.dataflow.DataFlow
+private import codeql.swift.dataflow.ExternalFlow
+private import codeql.swift.dataflow.FlowSteps
+
+/**
+ * A model for `Numeric` and related class members and functions that permit taint flow.
+ */
+private class NumericSummaries extends SummaryModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        ";;false;numericCast(_:);;;Argument[0];ReturnValue;taint",
+        ";;false;unsafeDowncast(_:to:);;;Argument[0];ReturnValue;taint",
+        ";;false;unsafeBitCast(_:to:);;;Argument[0];ReturnValue;taint",
+        ";Numeric;true;init(exactly:);;;Argument[0];ReturnValue.OptionalSome;value",
+        ";Numeric;true;init(bitPattern:);;;Argument[0];ReturnValue;taint",
+        ";BinaryInteger;true;init(_:);;;Argument[0];ReturnValue;taint",
+        ";BinaryInteger;true;init(clamping:);;;Argument[0];ReturnValue;taint",
+        ";BinaryInteger;true;init(truncatingIfNeeded:);;;Argument[0];ReturnValue;taint",
+        ";BinaryInteger;true;init(_:format:lenient:);;;Argument[0];ReturnValue;taint",
+        ";BinaryInteger;true;init(_:strategy:);;;Argument[0];ReturnValue;taint",
+        ";BinaryInteger;true;formatted();;;Argument[-1];ReturnValue;taint",
+        ";BinaryInteger;true;formatted(_:);;;Argument[-1];ReturnValue;taint",
+        ";FixedWidthInteger;true;init(_:radix:);;;Argument[0];ReturnValue;taint",
+        ";FixedWidthInteger;true;init(littleEndian:);;;Argument[0];ReturnValue;taint",
+        ";FixedWidthInteger;true;init(bigEndian:);;;Argument[0];ReturnValue;taint",
+        ";FloatingPoint;true;init(_:);;;Argument[0];ReturnValue;taint",
+        ";FloatingPoint;true;init(sign:exponent:significand:);;;Argument[1..2];ReturnValue;taint",
+        ";FloatingPoint;true;init(signOf:magnitudeOf:);;;Argument[1];ReturnValue;taint",
+      ]
+  }
+}
+
+/**
+ * A content implying that, if a `Numeric` is tainted, then some of its fields are
+ * tainted.
+ */
+private class NumericFieldsInheritTaint extends TaintInheritingContent,
+  DataFlow::Content::FieldContent
+{
+  NumericFieldsInheritTaint() {
+    this.getField().hasQualifiedName("FixedWidthInteger", ["littleEndian", "bigEndian"])
+    or
+    this.getField()
+        .hasQualifiedName(["Double", "Float", "Float80", "FloatingPoint"],
+          ["exponent", "significand"])
+  }
+}

swift/ql/lib/codeql/swift/frameworks/StandardLibrary/StandardLibrary.qll

@@ -15,6 +15,7 @@ private import NsData
 private import NsObject
 private import NsString
 private import NsUrl
+private import Numeric
 private import PointerTypes
 private import Sequence
 private import Set

swift/ql/lib/codeql/swift/frameworks/StandardLibrary/String.qll

@@ -27,7 +27,7 @@ private class StringSource extends SourceModelCsv {
 }
 
 /**
- * A model for `String` and `StringProtocol` members that permit taint flow.
+ * A model for members of `String`, `StringProtocol` and similar classes that permit taint flow.
  */
 private class StringSummaries extends SummaryModelCsv {
   override predicate row(string row) {
@@ -124,7 +124,8 @@ private class StringSummaries extends SummaryModelCsv {
         ";String;true;randomElement();;;Argument[-1];ReturnValue;taint",
         ";String;true;randomElement(using:);;;Argument[-1];ReturnValue;taint",
         ";String;true;enumerated();;;Argument[-1];ReturnValue;taint",
-        ";String;true;encode(to:);;;Argument[-1];Argument[0];taint"
+        ";String;true;encode(to:);;;Argument[-1];Argument[0];taint",
+        ";LosslessStringConvertible;true;init(_:);;;Argument[0];ReturnValue;taint",
       ]
   }
 }

swift/ql/lib/codeql/swift/security/CommandInjectionExtensions.qll

@@ -69,3 +69,13 @@ private class CommandInjectionSinks extends SinkModelCsv {
       ]
   }
 }
+
+/**
+ * A barrier for command injection vulnerabilities.
+ */
+private class CommandInjectionDefaultBarrier extends CommandInjectionBarrier {
+  CommandInjectionDefaultBarrier() {
+    // any numeric type
+    this.asExpr().getType().getUnderlyingType().getABaseType*().getName() = "Numeric"
+  }
+}

swift/ql/lib/codeql/swift/security/PredicateInjectionExtensions.qll

@@ -39,3 +39,13 @@ private class PredicateInjectionSinkCsv extends SinkModelCsv {
       ]
   }
 }
+
+/**
+ * A barrier for predicate injection vulnerabilities vulnerabilities.
+ */
+private class PredicateInjectionDefaultBarrier extends PredicateInjectionBarrier {
+  PredicateInjectionDefaultBarrier() {
+    // any numeric type
+    this.asExpr().getType().getUnderlyingType().getABaseType*().getName() = "Numeric"
+  }
+}

swift/ql/lib/codeql/swift/security/SqlInjectionExtensions.qll

@@ -153,3 +153,13 @@ private class GrdbDefaultSqlInjectionSink extends SqlInjectionSink {
 private class DefaultSqlInjectionSink extends SqlInjectionSink {
   DefaultSqlInjectionSink() { sinkNode(this, "sql-injection") }
 }
+
+/**
+ * A barrier for SQL injection.
+ */
+private class SqlInjectionDefaultBarrier extends SqlInjectionBarrier {
+  SqlInjectionDefaultBarrier() {
+    // any numeric type
+    this.asExpr().getType().getUnderlyingType().getABaseType*().getName() = "Numeric"
+  }
+}

swift/ql/lib/codeql/swift/security/UncontrolledFormatStringExtensions.qll

@@ -42,3 +42,13 @@ private class DefaultUncontrolledFormatStringSink extends UncontrolledFormatStri
     sinkNode(this, "format-string")
   }
 }
+
+/**
+ * A barrier for uncontrolled format string vulnerabilities.
+ */
+private class UncontrolledFormatStringDefaultBarrier extends UncontrolledFormatStringBarrier {
+  UncontrolledFormatStringDefaultBarrier() {
+    // any numeric type
+    this.asExpr().getType().getUnderlyingType().getABaseType*().getName() = "Numeric"
+  }
+}

swift/ql/lib/codeql/swift/security/UnsafeJsEvalExtensions.qll

@@ -30,7 +30,7 @@ class UnsafeJsEvalAdditionalFlowStep extends Unit {
 }
 
 /**
- * A default SQL injection sink for the `WKWebView` interface.
+ * A default javascript evaluation sink for the `WKWebView` interface.
  */
 private class WKWebViewDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
   WKWebViewDefaultUnsafeJsEvalSink() {
@@ -50,7 +50,7 @@ private class WKWebViewDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
 }
 
 /**
- * A default SQL injection sink for the `WKUserContentController` interface.
+ * A default javascript evaluation sink for the `WKUserContentController` interface.
  */
 private class WKUserContentControllerDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
   WKUserContentControllerDefaultUnsafeJsEvalSink() {
@@ -61,7 +61,7 @@ private class WKUserContentControllerDefaultUnsafeJsEvalSink extends UnsafeJsEva
 }
 
 /**
- * A default SQL injection sink for the `UIWebView` and `WebView` interfaces.
+ * A default javascript evaluation sink for the `UIWebView` and `WebView` interfaces.
  */
 private class UIWebViewDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
   UIWebViewDefaultUnsafeJsEvalSink() {
@@ -74,7 +74,7 @@ private class UIWebViewDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
 }
 
 /**
- * A default SQL injection sink for the `JSContext` interface.
+ * A default javascript evaluation sink for the `JSContext` interface.
  */
 private class JSContextDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
   JSContextDefaultUnsafeJsEvalSink() {
@@ -87,7 +87,7 @@ private class JSContextDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
 }
 
 /**
- * A default SQL injection sink for the `JSEvaluateScript` function.
+ * A default javascript evaluation sink for the `JSEvaluateScript` function.
  */
 private class JSEvaluateScriptDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
   JSEvaluateScriptDefaultUnsafeJsEvalSink() {
@@ -98,7 +98,7 @@ private class JSEvaluateScriptDefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
 }
 
 /**
- * A default SQL injection additional taint step.
+ * A default javascript evaluation additional taint step.
  */
 private class DefaultUnsafeJsEvalAdditionalFlowStep extends UnsafeJsEvalAdditionalFlowStep {
   override predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
@@ -120,3 +120,13 @@ private class DefaultUnsafeJsEvalAdditionalFlowStep extends UnsafeJsEvalAddition
 private class DefaultUnsafeJsEvalSink extends UnsafeJsEvalSink {
   DefaultUnsafeJsEvalSink() { sinkNode(this, "code-injection") }
 }
+
+/**
+ * A barrier for javascript evaluation.
+ */
+private class UnsafeJsEvalDefaultBarrier extends UnsafeJsEvalBarrier {
+  UnsafeJsEvalDefaultBarrier() {
+    // any numeric type
+    this.asExpr().getType().getUnderlyingType().getABaseType*().getName() = "Numeric"
+  }
+}

swift/ql/lib/codeql/swift/security/regex/RegexInjectionExtensions.qll

@@ -57,3 +57,13 @@ private class RegexInjectionSinks extends SinkModelCsv {
       ]
   }
 }
+
+/**
+ * A barrier for regular expression injection vulnerabilities.
+ */
+private class RegexInjectionDefaultBarrier extends RegexInjectionBarrier {
+  RegexInjectionDefaultBarrier() {
+    // any numeric type
+    this.asExpr().getType().getUnderlyingType().getABaseType*().getName() = "Numeric"
+  }
+}

swift/ql/src/change-notes/2023-09-19-numeric-barriers.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Adder barriers for numeric type values to the injection-like queries, to reduce false positive results where the user input that can be injected is constrainted to a numerical value. The queries updated by this change are: "Predicate built from user-controlled sources" (`swift/predicate-injection`), "Database query built from user-controlled sources" (`swift/sql-injection`), "Uncontrolled format string" (`swift/uncontrolled-format-string`), "JavaScript Injection" (`swift/unsafe-js-eval`) and "Regular expression injection" (`swift/regex-injection`).