Merge branch 'github:main' into UnsafeUnpack

Sim4n6 · web-flow · commit 7a126a2317e7 · 2023-01-27T16:09:41.000+01:00
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected b/cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
diff --git a/cpp/ql/test/library-tests/dataflow/taint-tests/standalone_iterators.cpp b/cpp/ql/test/library-tests/dataflow/taint-tests/standalone_iterators.cpp
@@ -27,6 +27,10 @@ class int_iterator_by_trait {
 template<>
 struct std::iterator_traits<int_iterator_by_trait> {
     typedef input_iterator_tag iterator_category;
+    typedef int value_type;
+    typedef size_t difference_type;
+    typedef int* pointer;
+    typedef int& reference;
 };
 
 class non_iterator {
@@ -69,6 +73,10 @@ class insert_iterator_by_trait {
 template<>
 struct std::iterator_traits<insert_iterator_by_trait> {
     typedef output_iterator_tag iterator_category;
+    typedef int value_type;
+    typedef size_t difference_type;
+    typedef int* pointer;
+    typedef int& reference;
 };
 
 class container {
diff --git a/docs/codeql/writing-codeql-queries/metadata-for-codeql-queries.rst b/docs/codeql/writing-codeql-queries/metadata-for-codeql-queries.rst
@@ -45,7 +45,7 @@ The following properties are supported by all query files:
 |                       | | ``high``                |                                                                                                                                                                                                                                                                                                                                                                       |
 |                       | | ``very-high``           |                                                                                                                                                                                                                                                                                                                                                                       |
 +-----------------------+---------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
-| ``@problem.severity`` | | ``error``               | Defines the level of severity of any alerts generated by a non-security query. This, along with the ``@precision`` property, determines how the results are displayed on GitHub.                                                                                                                                                                                      |
+| ``@problem.severity`` | | ``error``               | Defines the level of severity of any alerts generated by a non-security query. This, along with the ``@precision`` property, determines how the results are displayed on GitHub. For more information, see the `Query metadata style guide <https://github.com/github/codeql/blob/main/docs/query-metadata-style-guide.md>`__.                                        |
 |                       | | ``warning``             |                                                                                                                                                                                                                                                                                                                                                                       |
 |                       | | ``recommendation``      |                                                                                                                                                                                                                                                                                                                                                                       |
 +-----------------------+---------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
diff --git a/go/extractor/util/util.go b/go/extractor/util/util.go
@@ -170,7 +170,7 @@ func GetPkgDir(pkgpath string, flags ...string) string {
 // DepErrors checks there are any errors resolving dependencies for `pkgpath`. It passes the `go
 // list` command the flags specified by `flags`.
 func DepErrors(pkgpath string, flags ...string) bool {
-	out, err := runGoList("{{if .DepsErrors}}{{else}}error{{end}}", []string{pkgpath}, flags...)
+	out, err := runGoList("{{if .DepsErrors}}error{{else}}{{end}}", []string{pkgpath}, flags...)
 	if err != nil {
 		// if go list failed, assume dependencies are broken
 		return false
diff --git a/ruby/Cargo.lock b/ruby/Cargo.lock
diff --git a/ruby/extractor/Cargo.toml b/ruby/extractor/Cargo.toml
@@ -16,7 +16,7 @@ clap = "3.0"
 tracing = "0.1"
 tracing-subscriber = { version = "0.3.3", features = ["env-filter"] }
 rayon = "1.5.0"
-num_cpus = "1.13.0"
+num_cpus = "1.14.0"
 regex = "1.7.1"
 encoding = "0.2"
 lazy_static = "1.4.0"
diff --git a/ruby/ql/lib/change-notes/2023-01-12-rack.md b/ruby/ql/lib/change-notes/2023-01-12-rack.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Access to headers stored in the `env` of Rack requests is now recognized as a source of remote input.
diff --git a/ruby/ql/lib/codeql/ruby/Frameworks.qll b/ruby/ql/lib/codeql/ruby/Frameworks.qll
@@ -16,6 +16,7 @@ private import codeql.ruby.frameworks.ActiveSupport
 private import codeql.ruby.frameworks.Archive
 private import codeql.ruby.frameworks.Arel
 private import codeql.ruby.frameworks.GraphQL
+private import codeql.ruby.frameworks.Rack
 private import codeql.ruby.frameworks.Rails
 private import codeql.ruby.frameworks.Railties
 private import codeql.ruby.frameworks.Stdlib
diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll
@@ -1002,6 +1002,9 @@ class CallableNode extends ExprNode {
   /** Gets the `n`th positional parameter. */
   ParameterNode getParameter(int n) { this.getParameterPosition(result).isPositional(n) }
 
+  /** Gets the number of positional parameters of this callable. */
+  final int getNumberOfParameters() { result = count(this.getParameter(_)) }
+
   /** Gets the keyword parameter of the given name. */
   ParameterNode getKeywordParameter(string name) {
     this.getParameterPosition(result).isKeyword(name)
diff --git a/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll b/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll
@@ -301,27 +301,39 @@ private module Request {
     override Http::Server::RequestInputKind getKind() { result = Http::Server::bodyInputKind() }
   }
 
-  /**
-   * A method call on `request` which returns the rack env.
-   * This is a hash containing all the information about the request. Values
-   * under keys starting with `HTTP_` are user-controlled.
-   */
-  private class EnvCall extends RequestMethodCall {
-    EnvCall() { this.getMethodName() = ["env", "filtered_env"] }
-  }
+  private module Env {
+    abstract private class Env extends DataFlow::LocalSourceNode { }
+
+    /**
+     * A method call on `request` which returns the rack env.
+     * This is a hash containing all the information about the request. Values
+     * under keys starting with `HTTP_` are user-controlled.
+     */
+    private class RequestEnvCall extends DataFlow::CallNode, Env {
+      RequestEnvCall() { this.getMethodName() = ["env", "filtered_env"] }
+    }
 
-  /**
-   * A read of a user-controlled parameter from the request env.
-   */
-  private class EnvHttpAccess extends DataFlow::CallNode, Http::Server::RequestInputAccess::Range {
-    EnvHttpAccess() {
-      this = any(EnvCall c).getAMethodCall("[]") and
-      this.getArgument(0).getConstantValue().getString().regexpMatch("^HTTP_.+")
+    private import codeql.ruby.frameworks.Rack
+
+    private class RackEnv extends Env {
+      RackEnv() { this = any(Rack::AppCandidate app).getEnv().getALocalUse() }
     }
 
-    override Http::Server::RequestInputKind getKind() { result = Http::Server::headerInputKind() }
+    /**
+     * A read of a user-controlled parameter from the request env.
+     */
+    private class EnvHttpAccess extends DataFlow::CallNode, Http::Server::RequestInputAccess::Range {
+      EnvHttpAccess() {
+        this = any(Env c).getAMethodCall("[]") and
+        exists(string key | key = this.getArgument(0).getConstantValue().getString() |
+          key.regexpMatch("^HTTP_.+") or key = "PATH_INFO"
+        )
+      }
 
-    override string getSourceType() { result = "ActionDispatch::Request#env[]" }
+      override Http::Server::RequestInputKind getKind() { result = Http::Server::headerInputKind() }
+
+      override string getSourceType() { result = "ActionDispatch::Request#env[]" }
+    }
   }
 }
 
diff --git a/ruby/ql/lib/codeql/ruby/frameworks/Rack.qll b/ruby/ql/lib/codeql/ruby/frameworks/Rack.qll
@@ -0,0 +1,48 @@
+/**
+ * Provides modeling for the Rack library.
+ */
+
+private import codeql.ruby.controlflow.CfgNodes::ExprNodes
+private import codeql.ruby.DataFlow
+private import codeql.ruby.typetracking.TypeTracker
+
+/**
+ * Provides modeling for the Rack library.
+ */
+module Rack {
+  /**
+   * A class that may be a rack application.
+   * This is a class that has a `call` method that takes a single argument
+   * (traditionally called `env`) and returns a rack-compatible response.
+   */
+  class AppCandidate extends DataFlow::ClassNode {
+    private DataFlow::MethodNode call;
+
+    AppCandidate() {
+      call = this.getInstanceMethod("call") and
+      call.getNumberOfParameters() = 1 and
+      call.getReturn() = trackRackResponse()
+    }
+
+    /**
+     * Gets the environment of the request, which is the lone parameter to the `call` method.
+     */
+    DataFlow::ParameterNode getEnv() { result = call.getParameter(0) }
+  }
+
+  private predicate isRackResponse(DataFlow::Node r) {
+    // [status, headers, body]
+    r.asExpr().(ArrayLiteralCfgNode).getNumberOfArguments() = 3
+  }
+
+  private DataFlow::LocalSourceNode trackRackResponse(TypeTracker t) {
+    t.start() and
+    isRackResponse(result)
+    or
+    exists(TypeTracker t2 | result = trackRackResponse(t2).track(t2, t))
+  }
+
+  private DataFlow::Node trackRackResponse() {
+    trackRackResponse(TypeTracker::end()).flowsTo(result)
+  }
+}
diff --git a/ruby/ql/test/library-tests/frameworks/rack/Rack.expected b/ruby/ql/test/library-tests/frameworks/rack/Rack.expected
@@ -0,0 +1,4 @@
+| rack.rb:1:1:5:3 | HelloWorld | rack.rb:2:12:2:14 | env |
+| rack.rb:7:1:16:3 | Proxy | rack.rb:12:12:12:18 | the_env |
+| rack.rb:18:1:31:3 | Logger | rack.rb:24:12:24:14 | env |
+| rack.rb:45:1:61:3 | Baz | rack.rb:46:12:46:14 | env |
diff --git a/ruby/ql/test/library-tests/frameworks/rack/Rack.ql b/ruby/ql/test/library-tests/frameworks/rack/Rack.ql
@@ -0,0 +1,4 @@
+private import codeql.ruby.frameworks.Rack
+private import codeql.ruby.DataFlow
+
+query predicate rackApps(Rack::AppCandidate c, DataFlow::ParameterNode env) { env = c.getEnv() }
diff --git a/ruby/ql/test/library-tests/frameworks/rack/rack.rb b/ruby/ql/test/library-tests/frameworks/rack/rack.rb
@@ -0,0 +1,61 @@
+class HelloWorld
+  def call(env)
+    [200, {'Content-Type' => 'text/plain'}, ['Hello World']]
+  end
+end
+
+class Proxy
+  def initialize(app)
+    @app = app
+  end
+
+  def call(the_env)
+    status, headers, body = @app.call(the_env)
+    [status, headers, body]
+  end
+end
+
+class Logger
+  def initialize(app, logger = nil)
+    @app = app
+    @logger = logger
+  end
+
+  def call(env)
+    began_at = Utils.clock_time
+    status, header, body = @app.call(env)
+    header = Utils::HeaderHash.new(header)
+    body = BodyProxy.new(body) { log(env, status, header, began_at) }
+    [status, header, body]
+  end
+end
+
+class Foo
+  def not_call(env)
+    [1, 2, 3]
+  end
+end
+
+class Bar
+  def call(env)
+    nil
+  end
+end
+
+class Baz
+  def call(env)
+    run(env)
+  end
+
+  def run(env)
+    if env[:foo] == "foo"
+      [200, {}, "foo"]
+    else
+      error
+    end
+  end
+
+  def error
+    [400, {}, "nope"]
+  end
+end
diff --git a/swift/ql/lib/codeql/swift/dataflow/ExternalFlow.qll b/swift/ql/lib/codeql/swift/dataflow/ExternalFlow.qll
@@ -89,9 +89,9 @@ private module Frameworks {
   private import codeql.swift.frameworks.StandardLibrary.UrlSession
   private import codeql.swift.frameworks.StandardLibrary.WebView
   private import codeql.swift.frameworks.Alamofire.Alamofire
-  private import codeql.swift.security.CleartextLogging
-  private import codeql.swift.security.PathInjection
-  private import codeql.swift.security.PredicateInjection
+  private import codeql.swift.security.CleartextLoggingExtensions
+  private import codeql.swift.security.PathInjectionExtensions
+  private import codeql.swift.security.PredicateInjectionExtensions
 }
 
 /**
diff --git a/swift/ql/lib/codeql/swift/security/CleartextLoggingExtensions.qll b/swift/ql/lib/codeql/swift/security/CleartextLoggingExtensions.qll
@@ -25,6 +25,9 @@ class CleartextLoggingAdditionalTaintStep extends Unit {
   abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
 }
 
+/**
+ * A sink defined in a CSV model.
+ */
 private class DefaultCleartextLoggingSink extends CleartextLoggingSink {
   DefaultCleartextLoggingSink() { sinkNode(this, "logging") }
 }
diff --git a/swift/ql/lib/codeql/swift/security/CleartextLoggingQuery.qll b/swift/ql/lib/codeql/swift/security/CleartextLoggingQuery.qll
@@ -6,7 +6,7 @@
 import swift
 private import codeql.swift.dataflow.DataFlow
 private import codeql.swift.dataflow.TaintTracking
-private import codeql.swift.security.CleartextLogging
+private import codeql.swift.security.CleartextLoggingExtensions
 private import codeql.swift.security.SensitiveExprs
 
 /**
diff --git a/swift/ql/lib/codeql/swift/security/PathInjectionExtensions.qll b/swift/ql/lib/codeql/swift/security/PathInjectionExtensions.qll
@@ -29,6 +29,9 @@ class PathInjectionAdditionalTaintStep extends Unit {
   abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
 }
 
+/**
+ * A sink defined in a CSV model.
+ */
 private class DefaultPathInjectionSink extends PathInjectionSink {
   DefaultPathInjectionSink() { sinkNode(this, "path-injection") }
 }
diff --git a/swift/ql/lib/codeql/swift/security/PathInjectionQuery.qll b/swift/ql/lib/codeql/swift/security/PathInjectionQuery.qll
@@ -8,7 +8,7 @@ private import codeql.swift.dataflow.DataFlow
 private import codeql.swift.dataflow.ExternalFlow
 private import codeql.swift.dataflow.FlowSources
 private import codeql.swift.dataflow.TaintTracking
-private import codeql.swift.security.PathInjection
+private import codeql.swift.security.PathInjectionExtensions
 
 /**
  * A taint-tracking configuration for path injection vulnerabilities.
diff --git a/swift/ql/lib/codeql/swift/security/PredicateInjectionExtensions.qll b/swift/ql/lib/codeql/swift/security/PredicateInjectionExtensions.qll
@@ -24,6 +24,9 @@ class PredicateInjectionAdditionalTaintStep extends Unit {
   abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
 }
 
+/**
+ * A sink defined in a CSV model.
+ */
 private class DefaultPredicateInjectionSink extends PredicateInjectionSink {
   DefaultPredicateInjectionSink() { sinkNode(this, "predicate-injection") }
 }
diff --git a/swift/ql/lib/codeql/swift/security/PredicateInjectionQuery.qll b/swift/ql/lib/codeql/swift/security/PredicateInjectionQuery.qll
diff --git a/swift/ql/lib/codeql/swift/security/UncontrolledFormatStringExtensions.qll b/swift/ql/lib/codeql/swift/security/UncontrolledFormatStringExtensions.qll
diff --git a/swift/ql/lib/codeql/swift/security/XXEExtensions.qll b/swift/ql/lib/codeql/swift/security/XXEExtensions.qll
diff --git a/swift/ql/lib/codeql/swift/security/XXEQuery.qll b/swift/ql/lib/codeql/swift/security/XXEQuery.qll
diff --git a/swift/ql/src/queries/Security/CWE-1204/StaticInitializationVector.ql b/swift/ql/src/queries/Security/CWE-1204/StaticInitializationVector.ql
diff --git a/swift/ql/test/query-tests/Security/CWE-1204/StaticInitializationVector.expected b/swift/ql/test/query-tests/Security/CWE-1204/StaticInitializationVector.expected
diff --git a/swift/ql/test/query-tests/Security/CWE-1204/rncryptor.swift b/swift/ql/test/query-tests/Security/CWE-1204/rncryptor.swift

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +---
 +category: minorAnalysis
 +---
 +* Access to headers stored in the `env` of Rack requests is now recognized as a source of remote input.