+
+
+
+When a library function dynamically constructs code in a potentially unsafe way,
+it's important to document to clients of the library that the function should only be
+used with trusted inputs.
+
+If the function is not documented as being potentially unsafe, then a client may
+incorrectly use inputs containing unsafe code fragments, and thereby leave the
+client vulnerable to code-injection attacks.
+
+
+
+
+
+
+Properly document library functions that construct code from unsanitized
+inputs, or avoid constructing code in the first place.
+
+
+
+
+
+The following example shows two methods implemented using eval: a simple
+deserialization routine and a getter method.
+If untrusted inputs are used with these methods,
+then an attacker might be able to execute arbitrary code on the system.
+
+
+
+
+
+To avoid this problem, either properly document that the function is potentially
+unsafe, or use an alternative solution such as JSON.parse or another library
+that does not allow arbitrary code to be executed.
+
+
+
+
+
+
+
+
+As another example, consider the below code which dynamically constructs
+a class that has a getter method with a custom name.
+
+
+
+
+
+The example dynamically constructs a string which is then executed using module_eval.
+This code will break if the specified name is not a valid Ruby identifier, and
+if the value is controlled by an attacker, then this could lead to code-injection.
+
+
+
+A more robust implementation, that is also immune to code-injection,
+can be made by using module_eval with a block and using define_method
+to define the getter method.
+
+
+
+
+
+
+
+This example dynamically registers a method on another class which
+forwards its arguments to a target object. This approach uses
+module_eval and string interpolation to construct class variables
+and methods.
+
+
+
+
+
+A safer approach is to use class_variable_set and
+class_variable_get along with define_method. String
+interpolation is still used to construct the class variable name, but this is
+safe because class_variable_set is not susceptible to code-injection.
+
+
+
+send is used to dynamically call the method specified by name.
+This is a more robust alternative than the previous example, because it does not allow
+arbitrary code to be executed, but it does still allow for any method to be called
+on the target object.
+
+
+
+
+
+
+
+OWASP:
+Code Injection.
+
+
+Wikipedia: Code Injection.
+
+
+Ruby documentation: define_method.
+
+
+Ruby documentation: class_variable_set.
+
+
+
diff --git a/ruby/ql/src/queries/security/cwe-094/UnsafeCodeConstruction.ql b/ruby/ql/src/queries/security/cwe-094/UnsafeCodeConstruction.ql
new file mode 100644
index 00000000000..6b7b923e934
--- /dev/null
+++ b/ruby/ql/src/queries/security/cwe-094/UnsafeCodeConstruction.ql
@@ -0,0 +1,23 @@
+/**
+ * @name Unsafe code constructed from library input
+ * @description Using externally controlled strings to construct code may allow a malicious
+ * user to execute arbitrary code.
+ * @kind path-problem
+ * @problem.severity warning
+ * @security-severity 6.1
+ * @precision medium
+ * @id rb/unsafe-code-construction
+ * @tags security
+ * external/cwe/cwe-094
+ * external/cwe/cwe-079
+ * external/cwe/cwe-116
+ */
+
+import codeql.ruby.security.UnsafeCodeConstructionQuery
+import DataFlow::PathGraph
+
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, Sink sinkNode
+where cfg.hasFlowPath(source, sink) and sinkNode = sink.getNode()
+select sink.getNode(), source, sink,
+ "This " + sinkNode.getSinkType() + " which depends on $@ is later $@.", source.getNode(),
+ "library input", sinkNode.getCodeSink(), "interpreted as code"
diff --git a/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction.rb b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction.rb
new file mode 100644
index 00000000000..d900a950a50
--- /dev/null
+++ b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction.rb
@@ -0,0 +1,10 @@
+module MyLib
+ def unsafeDeserialize(value)
+ eval("foo = #{value}")
+ foo
+ end
+
+ def unsafeGetter(obj, path)
+ eval("obj.#{path}")
+ end
+end
diff --git a/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction2.rb b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction2.rb
new file mode 100644
index 00000000000..3452dfab665
--- /dev/null
+++ b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction2.rb
@@ -0,0 +1,17 @@
+require 'json'
+
+module BadMakeGetter
+ # Makes a class with a method named `getter_name` that returns `val`
+ def self.define_getter_class(getter_name, val)
+ new_class = Class.new
+ new_class.module_eval <<-END
+ def #{getter_name}
+ #{JSON.dump(val)}
+ end
+ END
+ new_class
+ end
+end
+
+one = BadMakeGetter.define_getter_class(:one, "foo")
+puts "One is #{one.new.one}"
\ No newline at end of file
diff --git a/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction2Safe.rb b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction2Safe.rb
new file mode 100644
index 00000000000..6c64684b183
--- /dev/null
+++ b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction2Safe.rb
@@ -0,0 +1,13 @@
+# Uses `define_method` instead of constructing a string
+module GoodMakeGetter
+ def self.define_getter_class(getter_name, val)
+ new_class = Class.new
+ new_class.module_eval do
+ define_method(getter_name) { val }
+ end
+ new_class
+ end
+end
+
+two = GoodMakeGetter.define_getter_class(:two, "bar")
+puts "Two is #{two.new.two}"
diff --git a/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction3.rb b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction3.rb
new file mode 100644
index 00000000000..e1a81bafe52
--- /dev/null
+++ b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction3.rb
@@ -0,0 +1,11 @@
+module Invoker
+ def attach(klass, name, target)
+ klass.module_eval <<-CODE
+ @@#{name} = target
+
+ def #{name}(*args)
+ @@#{name}.#{name}(*args)
+ end
+ CODE
+ end
+end
diff --git a/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction3Safe.rb b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction3Safe.rb
new file mode 100644
index 00000000000..ac32224ce72
--- /dev/null
+++ b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction3Safe.rb
@@ -0,0 +1,9 @@
+module Invoker
+ def attach(klass, name, target)
+ var = :"@@#{name}"
+ klass.class_variable_set(var, target)
+ klass.define_method(name) do |*args|
+ self.class.class_variable_get(var).send(name, *args)
+ end
+ end
+end
diff --git a/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstructionSafe.rb b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstructionSafe.rb
new file mode 100644
index 00000000000..aa2445ae234
--- /dev/null
+++ b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstructionSafe.rb
@@ -0,0 +1,11 @@
+require 'json'
+
+module MyLib
+ def safeDeserialize(value)
+ JSON.parse(value)
+ end
+
+ def safeGetter(obj, path)
+ obj.dig(*path.split("."))
+ end
+end
diff --git a/ruby/ql/test/query-tests/security/cwe-094/CodeInjection.expected b/ruby/ql/test/query-tests/security/cwe-094/CodeInjection/CodeInjection.expected
similarity index 100%
rename from ruby/ql/test/query-tests/security/cwe-094/CodeInjection.expected
rename to ruby/ql/test/query-tests/security/cwe-094/CodeInjection/CodeInjection.expected
diff --git a/ruby/ql/test/query-tests/security/cwe-094/CodeInjection.qlref b/ruby/ql/test/query-tests/security/cwe-094/CodeInjection/CodeInjection.qlref
similarity index 100%
rename from ruby/ql/test/query-tests/security/cwe-094/CodeInjection.qlref
rename to ruby/ql/test/query-tests/security/cwe-094/CodeInjection/CodeInjection.qlref
diff --git a/ruby/ql/test/query-tests/security/cwe-094/CodeInjection.rb b/ruby/ql/test/query-tests/security/cwe-094/CodeInjection/CodeInjection.rb
similarity index 100%
rename from ruby/ql/test/query-tests/security/cwe-094/CodeInjection.rb
rename to ruby/ql/test/query-tests/security/cwe-094/CodeInjection/CodeInjection.rb
diff --git a/ruby/ql/test/query-tests/security/cwe-094/UnsafeCodeConstruction/UnsafeCodeConstruction.expected b/ruby/ql/test/query-tests/security/cwe-094/UnsafeCodeConstruction/UnsafeCodeConstruction.expected
new file mode 100644
index 00000000000..5b7cd78cca3
--- /dev/null
+++ b/ruby/ql/test/query-tests/security/cwe-094/UnsafeCodeConstruction/UnsafeCodeConstruction.expected
@@ -0,0 +1,35 @@
+edges
+| impl/unsafeCode.rb:2:12:2:17 | target : | impl/unsafeCode.rb:3:17:3:25 | #{...} |
+| impl/unsafeCode.rb:7:12:7:12 | x : | impl/unsafeCode.rb:8:30:8:30 | x |
+| impl/unsafeCode.rb:12:12:12:12 | x : | impl/unsafeCode.rb:13:33:13:33 | x |
+| impl/unsafeCode.rb:28:17:28:22 | my_arr : | impl/unsafeCode.rb:29:10:29:15 | my_arr |
+| impl/unsafeCode.rb:32:21:32:21 | x : | impl/unsafeCode.rb:34:10:34:12 | arr |
+| impl/unsafeCode.rb:37:15:37:15 | x : | impl/unsafeCode.rb:40:10:40:12 | arr |
+| impl/unsafeCode.rb:37:15:37:15 | x : | impl/unsafeCode.rb:44:10:44:12 | arr |
+| impl/unsafeCode.rb:47:15:47:15 | x : | impl/unsafeCode.rb:49:9:49:12 | #{...} |
+nodes
+| impl/unsafeCode.rb:2:12:2:17 | target : | semmle.label | target : |
+| impl/unsafeCode.rb:3:17:3:25 | #{...} | semmle.label | #{...} |
+| impl/unsafeCode.rb:7:12:7:12 | x : | semmle.label | x : |
+| impl/unsafeCode.rb:8:30:8:30 | x | semmle.label | x |
+| impl/unsafeCode.rb:12:12:12:12 | x : | semmle.label | x : |
+| impl/unsafeCode.rb:13:33:13:33 | x | semmle.label | x |
+| impl/unsafeCode.rb:28:17:28:22 | my_arr : | semmle.label | my_arr : |
+| impl/unsafeCode.rb:29:10:29:15 | my_arr | semmle.label | my_arr |
+| impl/unsafeCode.rb:32:21:32:21 | x : | semmle.label | x : |
+| impl/unsafeCode.rb:34:10:34:12 | arr | semmle.label | arr |
+| impl/unsafeCode.rb:37:15:37:15 | x : | semmle.label | x : |
+| impl/unsafeCode.rb:40:10:40:12 | arr | semmle.label | arr |
+| impl/unsafeCode.rb:44:10:44:12 | arr | semmle.label | arr |
+| impl/unsafeCode.rb:47:15:47:15 | x : | semmle.label | x : |
+| impl/unsafeCode.rb:49:9:49:12 | #{...} | semmle.label | #{...} |
+subpaths
+#select
+| impl/unsafeCode.rb:3:17:3:25 | #{...} | impl/unsafeCode.rb:2:12:2:17 | target : | impl/unsafeCode.rb:3:17:3:25 | #{...} | This string interpolation which depends on $@ is later $@. | impl/unsafeCode.rb:2:12:2:17 | target | library input | impl/unsafeCode.rb:3:5:3:27 | call to eval | interpreted as code |
+| impl/unsafeCode.rb:8:30:8:30 | x | impl/unsafeCode.rb:7:12:7:12 | x : | impl/unsafeCode.rb:8:30:8:30 | x | This string format which depends on $@ is later $@. | impl/unsafeCode.rb:7:12:7:12 | x | library input | impl/unsafeCode.rb:8:5:8:32 | call to eval | interpreted as code |
+| impl/unsafeCode.rb:13:33:13:33 | x | impl/unsafeCode.rb:12:12:12:12 | x : | impl/unsafeCode.rb:13:33:13:33 | x | This string format which depends on $@ is later $@. | impl/unsafeCode.rb:12:12:12:12 | x | library input | impl/unsafeCode.rb:13:5:13:35 | call to eval | interpreted as code |
+| impl/unsafeCode.rb:29:10:29:15 | my_arr | impl/unsafeCode.rb:28:17:28:22 | my_arr : | impl/unsafeCode.rb:29:10:29:15 | my_arr | This array which depends on $@ is later $@. | impl/unsafeCode.rb:28:17:28:22 | my_arr | library input | impl/unsafeCode.rb:29:5:29:27 | call to eval | interpreted as code |
+| impl/unsafeCode.rb:34:10:34:12 | arr | impl/unsafeCode.rb:32:21:32:21 | x : | impl/unsafeCode.rb:34:10:34:12 | arr | This array which depends on $@ is later $@. | impl/unsafeCode.rb:32:21:32:21 | x | library input | impl/unsafeCode.rb:34:5:34:24 | call to eval | interpreted as code |
+| impl/unsafeCode.rb:40:10:40:12 | arr | impl/unsafeCode.rb:37:15:37:15 | x : | impl/unsafeCode.rb:40:10:40:12 | arr | This array which depends on $@ is later $@. | impl/unsafeCode.rb:37:15:37:15 | x | library input | impl/unsafeCode.rb:40:5:40:24 | call to eval | interpreted as code |
+| impl/unsafeCode.rb:44:10:44:12 | arr | impl/unsafeCode.rb:37:15:37:15 | x : | impl/unsafeCode.rb:44:10:44:12 | arr | This array which depends on $@ is later $@. | impl/unsafeCode.rb:37:15:37:15 | x | library input | impl/unsafeCode.rb:44:5:44:24 | call to eval | interpreted as code |
+| impl/unsafeCode.rb:49:9:49:12 | #{...} | impl/unsafeCode.rb:47:15:47:15 | x : | impl/unsafeCode.rb:49:9:49:12 | #{...} | This string interpolation which depends on $@ is later $@. | impl/unsafeCode.rb:47:15:47:15 | x | library input | impl/unsafeCode.rb:51:5:51:13 | call to eval | interpreted as code |
diff --git a/ruby/ql/test/query-tests/security/cwe-094/UnsafeCodeConstruction/UnsafeCodeConstruction.qlref b/ruby/ql/test/query-tests/security/cwe-094/UnsafeCodeConstruction/UnsafeCodeConstruction.qlref
new file mode 100644
index 00000000000..ec336901db5
--- /dev/null
+++ b/ruby/ql/test/query-tests/security/cwe-094/UnsafeCodeConstruction/UnsafeCodeConstruction.qlref
@@ -0,0 +1 @@
+queries/security/cwe-094/UnsafeCodeConstruction.ql
\ No newline at end of file
diff --git a/ruby/ql/test/query-tests/security/cwe-094/UnsafeCodeConstruction/impl/unsafeCode.rb b/ruby/ql/test/query-tests/security/cwe-094/UnsafeCodeConstruction/impl/unsafeCode.rb
new file mode 100644
index 00000000000..27dd1d81f01
--- /dev/null
+++ b/ruby/ql/test/query-tests/security/cwe-094/UnsafeCodeConstruction/impl/unsafeCode.rb
@@ -0,0 +1,53 @@
+class Foobar
+ def foo1(target)
+ eval("foo = #{target}") # NOT OK
+ end
+
+ # sprintf
+ def foo2(x)
+ eval(sprintf("foo = %s", x)) # NOT OK
+ end
+
+ # String#%
+ def foo3(x)
+ eval("foo = %{foo}" % {foo: x}) # NOT OK
+ end
+
+ def indirect_eval(x)
+ eval(x) # OK - no construction.
+ end
+
+ def send_stuff(x)
+ foo.send("foo_#{x}") # OK - attacker cannot control entire string.
+ end
+
+ def named_code(code)
+ eval("def \n #{code} \n end") # OK - parameter is named code
+ end
+
+ def joinStuff(my_arr)
+ eval(my_arr.join("\n")) # NOT OK
+ end
+
+ def joinWithElemt(x)
+ arr = [x, "foobar"]
+ eval(arr.join("\n")) # NOT OK
+ end
+
+ def pushArr(x, y)
+ arr = []
+ arr.push(x)
+ eval(arr.join("\n")) # NOT OK
+
+ arr2 = []
+ arr2 << y
+ eval(arr.join("\n")) # NOT OK
+ end
+
+ def hereDoc(x)
+ foo = <<~HERE
+ #{x}
+ HERE
+ eval(foo) # NOT OK
+ end
+end
diff --git a/ruby/ql/test/query-tests/security/cwe-094/UnsafeCodeConstruction/unsafe-code.gemspec b/ruby/ql/test/query-tests/security/cwe-094/UnsafeCodeConstruction/unsafe-code.gemspec
new file mode 100644
index 00000000000..ab9639a5fc6
--- /dev/null
+++ b/ruby/ql/test/query-tests/security/cwe-094/UnsafeCodeConstruction/unsafe-code.gemspec
@@ -0,0 +1,5 @@
+Gem::Specification.new do |s|
+ s.name = 'unsafe-code'
+ s.require_path = "impl"
+ end
+
\ No newline at end of file
diff --git a/ruby/tools/qltest.cmd b/ruby/tools/qltest.cmd
index e43fe47e3fa..00a223cae5c 100644
--- a/ruby/tools/qltest.cmd
+++ b/ruby/tools/qltest.cmd
@@ -8,6 +8,7 @@ type NUL && "%CODEQL_DIST%\codeql.exe" database index-files ^
--include=**/Gemfile ^
--size-limit=5m ^
--language=ruby ^
+ --working-dir=. ^
"%CODEQL_EXTRACTOR_RUBY_WIP_DATABASE%"
exit /b %ERRORLEVEL%
diff --git a/swift/ql/lib/codeql/swift/dataflow/ExternalFlow.qll b/swift/ql/lib/codeql/swift/dataflow/ExternalFlow.qll
index 89db67289f6..2e82ab20861 100644
--- a/swift/ql/lib/codeql/swift/dataflow/ExternalFlow.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/ExternalFlow.qll
@@ -89,6 +89,7 @@ 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
}
diff --git a/swift/ql/lib/codeql/swift/security/CleartextLogging.qll b/swift/ql/lib/codeql/swift/security/CleartextLogging.qll
new file mode 100644
index 00000000000..b3aa3ba414e
--- /dev/null
+++ b/swift/ql/lib/codeql/swift/security/CleartextLogging.qll
@@ -0,0 +1,103 @@
+/** Provides classes and predicates to reason about cleartext logging of sensitive data vulnerabilities. */
+
+import swift
+private import codeql.swift.dataflow.DataFlow
+private import codeql.swift.dataflow.ExternalFlow
+private import codeql.swift.security.SensitiveExprs
+
+/** A data flow sink for cleartext logging of sensitive data vulnerabilities. */
+abstract class CleartextLoggingSink extends DataFlow::Node { }
+
+/** A sanitizer for cleartext logging of sensitive data vulnerabilities. */
+abstract class CleartextLoggingSanitizer extends DataFlow::Node { }
+
+/**
+ * A unit class for adding additional taint steps.
+ *
+ * Extend this class to add additional taint steps that should apply to paths related to
+ * cleartext logging of sensitive data vulnerabilities.
+ */
+class CleartextLoggingAdditionalTaintStep extends Unit {
+ /**
+ * Holds if the step from `n1` to `n2` should be considered a taint
+ * step for flows related to cleartext logging of sensitive data vulnerabilities.
+ */
+ abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
+}
+
+private class DefaultCleartextLoggingSink extends CleartextLoggingSink {
+ DefaultCleartextLoggingSink() { sinkNode(this, "logging") }
+}
+
+/**
+ * A sanitizer for `OSLogMessage`s configured with the appropriate privacy option.
+ * Numeric and boolean arguments aren't redacted unless the `private` or `sensitive` options are used.
+ * Arguments of other types are always redacted unless the `public` option is used.
+ */
+private class OsLogPrivacyCleartextLoggingSanitizer extends CleartextLoggingSanitizer {
+ OsLogPrivacyCleartextLoggingSanitizer() {
+ exists(CallExpr c, AutoClosureExpr e |
+ c.getStaticTarget().getName().matches("appendInterpolation(_:%privacy:%)") and
+ c.getArgument(0).getExpr() = e and
+ this.asExpr() = e
+ |
+ e.getExpr().getType() instanceof OsLogNonRedactedType and
+ c.getArgumentWithLabel("privacy").getExpr().(OsLogPrivacyRef).isSafe()
+ or
+ not e.getExpr().getType() instanceof OsLogNonRedactedType and
+ not c.getArgumentWithLabel("privacy").getExpr().(OsLogPrivacyRef).isPublic()
+ )
+ }
+}
+
+/** A type that isn't redacted by default in an `OSLogMessage`. */
+private class OsLogNonRedactedType extends Type {
+ OsLogNonRedactedType() {
+ this.getName() = [["", "U"] + "Int" + ["", "8", "16", "32", "64"], "Double", "Float", "Bool"]
+ }
+}
+
+/** A reference to a field of `OsLogPrivacy`. */
+private class OsLogPrivacyRef extends MemberRefExpr {
+ string optionName;
+
+ OsLogPrivacyRef() {
+ exists(FieldDecl f | this.getMember() = f |
+ f.getEnclosingDecl().(NominalTypeDecl).getName() = "OSLogPrivacy" and
+ optionName = f.getName()
+ )
+ }
+
+ /** Holds if this is a safe privacy option (private or sensitive). */
+ predicate isSafe() { optionName = ["private", "sensitive"] }
+
+ /** Holds if this is a public (that is, unsafe) privacy option. */
+ predicate isPublic() { optionName = "public" }
+}
+
+private class LoggingSinks extends SinkModelCsv {
+ override predicate row(string row) {
+ row =
+ [
+ ";;false;print(_:separator:terminator:);;;Argument[0].ArrayElement;logging",
+ ";;false;print(_:separator:terminator:);;;Argument[1..2];logging",
+ ";;false;print(_:separator:terminator:toStream:);;;Argument[0].ArrayElement;logging",
+ ";;false;print(_:separator:terminator:toStream:);;;Argument[1..2];logging",
+ ";;false;NSLog(_:_:);;;Argument[0];logging",
+ ";;false;NSLog(_:_:);;;Argument[1].ArrayElement;logging",
+ ";;false;NSLogv(_:_:);;;Argument[0];logging",
+ ";;false;NSLogv(_:_:);;;Argument[1].ArrayElement;logging",
+ ";;false;vfprintf(_:_:_:);;;Agument[1..2];logging",
+ ";Logger;true;log(_:);;;Argument[0];logging",
+ ";Logger;true;log(level:_:);;;Argument[1];logging",
+ ";Logger;true;trace(_:);;;Argument[1];logging",
+ ";Logger;true;debug(_:);;;Argument[1];logging",
+ ";Logger;true;info(_:);;;Argument[1];logging",
+ ";Logger;true;notice(_:);;;Argument[1];logging",
+ ";Logger;true;warning(_:);;;Argument[1];logging",
+ ";Logger;true;error(_:);;;Argument[1];logging",
+ ";Logger;true;critical(_:);;;Argument[1];logging",
+ ";Logger;true;fault(_:);;;Argument[1];logging",
+ ]
+ }
+}
diff --git a/swift/ql/lib/codeql/swift/security/CleartextLoggingQuery.qll b/swift/ql/lib/codeql/swift/security/CleartextLoggingQuery.qll
new file mode 100644
index 00000000000..d67f66e74b9
--- /dev/null
+++ b/swift/ql/lib/codeql/swift/security/CleartextLoggingQuery.qll
@@ -0,0 +1,32 @@
+/**
+ * Provides a taint-tracking configuration for reasoning about cleartext logging of
+ * sensitive data vulnerabilities.
+ */
+
+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.SensitiveExprs
+
+/**
+ * A taint-tracking configuration for cleartext logging of sensitive data vulnerabilities.
+ */
+class CleartextLoggingConfiguration extends TaintTracking::Configuration {
+ CleartextLoggingConfiguration() { this = "CleartextLoggingConfiguration" }
+
+ override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveExpr }
+
+ override predicate isSink(DataFlow::Node sink) { sink instanceof CleartextLoggingSink }
+
+ override predicate isSanitizer(DataFlow::Node sanitizer) {
+ sanitizer instanceof CleartextLoggingSanitizer
+ }
+
+ // Disregard paths that contain other paths. This helps with performance.
+ override predicate isSanitizerIn(DataFlow::Node node) { this.isSource(node) }
+
+ override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
+ any(CleartextLoggingAdditionalTaintStep s).step(n1, n2)
+ }
+}
diff --git a/swift/ql/src/queries/Security/CWE-312/CleartextLogging.qhelp b/swift/ql/src/queries/Security/CWE-312/CleartextLogging.qhelp
new file mode 100644
index 00000000000..5959fe5ef8d
--- /dev/null
+++ b/swift/ql/src/queries/Security/CWE-312/CleartextLogging.qhelp
@@ -0,0 +1,46 @@
+
+
+
+
+
+Attackers could gain access to sensitive information that is logged unencrypted.
+
+
+
+
+
+Always make sure to encrypt or obfuscate sensitive information before you log it.
+
+
+
+Generally, you should decrypt sensitive information only at the point where it is necessary for it to be used in cleartext.
+
+
+
+Be aware that external processes often store the standard output and
+standard error streams of the application. This will include logged sensitive information.
+
+
+
+
+
+The following example code logs user credentials (in this case, their password)
+in plaintext:
+
+
+
+Instead, you should encrypt or obfuscate the credentials, or omit them entirely:
+
+
+
+
+
+
+M. Dowd, J. McDonald and J. Schuhm, The Art of Software Security Assessment, 1st Edition, Chapter 2 - 'Common Vulnerabilities of Encryption', p. 43. Addison Wesley, 2006.
+M. Howard and D. LeBlanc, Writing Secure Code, 2nd Edition, Chapter 9 - 'Protecting Secret Data', p. 299. Microsoft, 2002.
+OWASP: Password Plaintext Storage.
+
+
+
diff --git a/swift/ql/src/queries/Security/CWE-312/CleartextLogging.ql b/swift/ql/src/queries/Security/CWE-312/CleartextLogging.ql
new file mode 100644
index 00000000000..df5c128685f
--- /dev/null
+++ b/swift/ql/src/queries/Security/CWE-312/CleartextLogging.ql
@@ -0,0 +1,24 @@
+/**
+ * @name Cleartext logging of sensitive information
+ * @description Logging sensitive information in plaintext can
+ * expose it to an attacker.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.5
+ * @precision high
+ * @id swift/clear-text-logging
+ * @tags security
+ * external/cwe/cwe-312
+ * external/cwe/cwe-359
+ * external/cwe/cwe-532
+ */
+
+import swift
+import codeql.swift.dataflow.DataFlow
+import codeql.swift.security.CleartextLoggingQuery
+import DataFlow::PathGraph
+
+from DataFlow::PathNode src, DataFlow::PathNode sink
+where any(CleartextLoggingConfiguration c).hasFlowPath(src, sink)
+select sink.getNode(), src, sink, "This $@ is written to a log file.", src.getNode(),
+ "potentially sensitive information"
diff --git a/swift/ql/src/queries/Security/CWE-312/CleartextLoggingBad.swift b/swift/ql/src/queries/Security/CWE-312/CleartextLoggingBad.swift
new file mode 100644
index 00000000000..036001e8717
--- /dev/null
+++ b/swift/ql/src/queries/Security/CWE-312/CleartextLoggingBad.swift
@@ -0,0 +1,2 @@
+let password = "P@ssw0rd"
+NSLog("User password changed to \(password)")
diff --git a/swift/ql/src/queries/Security/CWE-312/CleartextLoggingGood.swift b/swift/ql/src/queries/Security/CWE-312/CleartextLoggingGood.swift
new file mode 100644
index 00000000000..1d90ac5e565
--- /dev/null
+++ b/swift/ql/src/queries/Security/CWE-312/CleartextLoggingGood.swift
@@ -0,0 +1,2 @@
+let password = "P@ssw0rd"
+NSLog("User password changed")
diff --git a/swift/ql/test/query-tests/Security/CWE-312/CleartextLoggingTest.expected b/swift/ql/test/query-tests/Security/CWE-312/CleartextLoggingTest.expected
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/swift/ql/test/query-tests/Security/CWE-312/CleartextLoggingTest.ql b/swift/ql/test/query-tests/Security/CWE-312/CleartextLoggingTest.ql
new file mode 100644
index 00000000000..d57dc15bbad
--- /dev/null
+++ b/swift/ql/test/query-tests/Security/CWE-312/CleartextLoggingTest.ql
@@ -0,0 +1,24 @@
+import swift
+import codeql.swift.dataflow.DataFlow
+import codeql.swift.security.CleartextLoggingQuery
+import TestUtilities.InlineExpectationsTest
+
+class CleartextLogging extends InlineExpectationsTest {
+ CleartextLogging() { this = "CleartextLogging" }
+
+ override string getARelevantTag() { result = "hasCleartextLogging" }
+
+ override predicate hasActualResult(Location location, string element, string tag, string value) {
+ exists(
+ CleartextLoggingConfiguration config, DataFlow::Node source, DataFlow::Node sink,
+ Expr sinkExpr
+ |
+ config.hasFlow(source, sink) and
+ sinkExpr = sink.asExpr() and
+ location = sinkExpr.getLocation() and
+ element = sinkExpr.toString() and
+ tag = "hasCleartextLogging" and
+ value = source.asExpr().getLocation().getStartLine().toString()
+ )
+ }
+}
diff --git a/swift/ql/test/query-tests/Security/CWE-312/cleartextLoggingTest.swift b/swift/ql/test/query-tests/Security/CWE-312/cleartextLoggingTest.swift
new file mode 100644
index 00000000000..9151ab0f44f
--- /dev/null
+++ b/swift/ql/test/query-tests/Security/CWE-312/cleartextLoggingTest.swift
@@ -0,0 +1,158 @@
+// --- stubs ---
+
+func NSLog(_ format: String, _ args: CVarArg...) {}
+func NSLogv(_ format: String, _ args: CVaListPointer) {}
+func getVaList(_ args: [CVarArg]) -> CVaListPointer { return CVaListPointer(_fromUnsafeMutablePointer: UnsafeMutablePointer(bitPattern: 0)!) }
+
+struct OSLogType : RawRepresentable {
+ static let `default` = OSLogType(rawValue: 0)
+ let rawValue: UInt8
+ init(rawValue: UInt8) { self.rawValue = rawValue}
+}
+
+struct OSLogStringAlignment {
+ static var none = OSLogStringAlignment()
+}
+
+enum OSLogIntegerFormatting { case decimal }
+enum OSLogInt32ExtendedFormat { case none }
+enum OSLogFloatFormatting { case fixed }
+enum OSLogBoolFormat { case truth }
+
+struct OSLogPrivacy {
+ enum Mask { case none }
+ static var auto = OSLogPrivacy()
+ static var `private` = OSLogPrivacy()
+ static var `public` = OSLogPrivacy()
+ static var sensitive = OSLogPrivacy()
+
+ static func auto(mask: OSLogPrivacy.Mask) -> OSLogPrivacy { return .auto }
+ static func `private`(mask: OSLogPrivacy.Mask) -> OSLogPrivacy { return .private }
+ static func `sensitive`(mask: OSLogPrivacy.Mask) -> OSLogPrivacy { return .sensitive }
+}
+
+struct OSLogInterpolation : StringInterpolationProtocol {
+ typealias StringLiteralType = String
+ init(literalCapacity: Int, interpolationCount: Int) {}
+ func appendLiteral(_: Self.StringLiteralType) {}
+ mutating func appendInterpolation(_ argumentString: @autoclosure @escaping () -> String, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ argumentString: @autoclosure @escaping () -> String, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto, attributes: String) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int8, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int16, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int32, format: OSLogInt32ExtendedFormat, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int32, format: OSLogInt32ExtendedFormat, privacy: OSLogPrivacy = .auto, attributes: String) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int32, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Int64, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> UInt, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> UInt8, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> UInt16, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> UInt32, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> UInt64, format: OSLogIntegerFormatting = .decimal, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Double, format: OSLogFloatFormatting = .fixed, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Double, format: OSLogFloatFormatting = .fixed, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto, attributes: String) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Float,format: OSLogFloatFormatting = .fixed, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto) {}
+ mutating func appendInterpolation(_ number: @autoclosure @escaping () -> Float, format: OSLogFloatFormatting = .fixed, align: OSLogStringAlignment = .none, privacy: OSLogPrivacy = .auto, attributes: String) {}
+ mutating func appendInterpolation(_ boolean: @autoclosure @escaping () -> Bool, format: OSLogBoolFormat = .truth, privacy: OSLogPrivacy = .auto) {}
+}
+
+struct OSLogMessage : ExpressibleByStringInterpolation {
+ typealias StringInterpolation = OSLogInterpolation
+ typealias StringLiteralType = String
+ typealias ExtendedGraphemeClusterLiteralType = String
+ typealias UnicodeScalarLiteralType = String
+ init(stringInterpolation: OSLogInterpolation) {}
+ init(stringLiteral: String) {}
+ init(extendedGraphemeClusterLiteral: String) {}
+ init(unicodeScalarLiteral: String) {}
+}
+
+struct Logger {
+ func log(_ message: OSLogMessage) {}
+ func log(level: OSLogType, _ message: OSLogMessage) {}
+ func notice(_: OSLogMessage) {}
+ func debug(_: OSLogMessage) {}
+ func trace(_: OSLogMessage) {}
+ func info(_: OSLogMessage) {}
+ func error(_: OSLogMessage) {}
+ func warning(_: OSLogMessage) {}
+ func fault(_: OSLogMessage) {}
+ func critical(_: OSLogMessage) {}
+
+}
+
+// --- tests ---
+
+func test1(password: String, passwordHash : String) {
+ print(password) // $ MISSING: hasCleartextLogging=87
+ print(password, separator: "") // $ MISSING: $ hasCleartextLogging=88
+ print("", separator: password) // $ hasCleartextLogging=89
+ print(password, separator: "", terminator: "") // $ MISSING: hasCleartextLogging=90
+ print("", separator: password, terminator: "") // $ hasCleartextLogging=91
+ print("", separator: "", terminator: password) // $ hasCleartextLogging=92
+ print(passwordHash) // Safe
+
+ NSLog(password) // $ hasCleartextLogging=95
+ NSLog("%@", password as! CVarArg) // $ MISSING: hasCleartextLogging=96
+ NSLog("%@ %@", "" as! CVarArg, password as! CVarArg) // $ MISSING: hasCleartextLogging=97
+ NSLog("\(password)") // $ hasCleartextLogging=98
+ NSLogv("%@", getVaList([password as! CVarArg])) // $ MISSING: hasCleartextLogging=99
+ NSLogv("%@ %@", getVaList(["" as! CVarArg, password as! CVarArg])) // $ MISSING: hasCleartextLogging=100
+ NSLog(passwordHash) // SAfe
+ NSLogv("%@", getVaList([passwordHash as! CVarArg])) // Safe
+
+ let bankAccount: Int = 0
+ let log = Logger()
+ // These MISSING test cases will be fixed when we properly generate the CFG around autoclosures.
+ log.log("\(password)") // Safe
+ log.log("\(password, privacy: .auto)") // Safe
+ log.log("\(password, privacy: .private)") // Safe
+ log.log("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=110
+ log.log("\(passwordHash, privacy: .public)") // Safe
+ log.log("\(password, privacy: .sensitive)") // Safe
+ log.log("\(bankAccount)") // $ MISSING: hasCleartextLogging=113
+ log.log("\(bankAccount, privacy: .auto)") // $ MISSING: hasCleartextLogging=114
+ log.log("\(bankAccount, privacy: .private)") // Safe
+ log.log("\(bankAccount, privacy: .public)") // $ MISSING: hasCleartextLogging=116
+ log.log("\(bankAccount, privacy: .sensitive)") // Safe
+ log.log(level: .default, "\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=118
+ log.trace("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=119
+ log.trace("\(passwordHash, privacy: .public)") // Safe
+ log.debug("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=121
+ log.debug("\(passwordHash, privacy: .public)") // Safe
+ log.info("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=123
+ log.info("\(passwordHash, privacy: .public)") // Safe
+ log.notice("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=125
+ log.notice("\(passwordHash, privacy: .public)") // Safe
+ log.warning("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=127
+ log.warning("\(passwordHash, privacy: .public)") // Safe
+ log.error("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=129
+ log.error("\(passwordHash, privacy: .public)") // Safe
+ log.critical("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=131
+ log.critical("\(passwordHash, privacy: .public)") // Safe
+ log.fault("\(password, privacy: .public)") // $ MISSING: hasCleartextLogging=133
+ log.fault("\(passwordHash, privacy: .public)") // Safe
+}
+
+class MyClass {
+ var harmless = "abc"
+ var password = "123"
+}
+
+func getPassword() -> String { return "" }
+func doSomething(password: String) { }
+
+func test3(x: String) {
+ // alternative evidence of sensitivity...
+
+ NSLog(x) // $ MISSING: hasCleartextLogging=148
+ doSomething(password: x);
+ NSLog(x) // $ hasCleartextLogging=149
+
+ let y = getPassword();
+ NSLog(y) // $ hasCleartextLogging=152
+
+ let z = MyClass()
+ NSLog(z.harmless) // Safe
+ NSLog(z.password) // $ hasCleartextLogging=157
+}