Ruby: Implement rb/clear-text-logging-sensitive-data

ruby/ql/lib/codeql/ruby/security/CleartextLoggingCustomizations.qll

@@ -0,0 +1,323 @@
+/**
+ * Provides default sources, sinks and sanitizers for reasoning about
+ * cleartext logging of sensitive information, as well as extension points for
+ * adding your own.
+ */
+
+private import ruby
+private import codeql.ruby.DataFlow
+private import codeql.ruby.TaintTracking::TaintTracking
+private import codeql.ruby.Concepts
+private import codeql.ruby.dataflow.RemoteFlowSources
+private import internal.SensitiveDataHeuristics::HeuristicNames
+private import codeql.ruby.CFG
+private import codeql.ruby.dataflow.SSA
+
+module CleartextLogging {
+  /**
+   * A data flow source for cleartext logging of sensitive information.
+   */
+  abstract class Source extends DataFlow::Node {
+    /** Gets a string that describes the type of this data flow source. */
+    abstract string describe();
+  }
+
+  /**
+   * A data flow sink for cleartext logging of sensitive information.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for cleartext logging of sensitive information.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /**
+   * A node that receives sanitized sensitive information.
+   */
+  abstract class SanitizerIn extends DataFlow::Node { }
+
+  /**
+   * Holds if `re` may be a regular expression that can be used to sanitize
+   * sensitive data with a call to `sub`.
+   */
+  private predicate effectiveSubRegExp(RegExpLiteral re) {
+    re.getConstantValue().getStringOrSymbol().matches([".*", ".+"])
+  }
+
+  /**
+   * Holds if `re` may be a regular expression that can be used to sanitize
+   * sensitive data with a call to `gsub`.
+   */
+  private predicate effectiveGsubRegExp(RegExpLiteral re) {
+    re.getConstantValue().getStringOrSymbol().matches(".")
+  }
+
+  /**
+   * A call to `sub`/`sub!` or `gsub`/`gsub!` that seems to mask sensitive information.
+   */
+  private class MaskingReplacerSanitizer extends Sanitizer, DataFlow::CallNode {
+    MaskingReplacerSanitizer() {
+      exists(RegExpLiteral re |
+        re = this.getArgument(0).asExpr().getExpr() and
+        (
+          this.getMethodName() = ["sub", "sub!"] and effectiveSubRegExp(re)
+          or
+          this.getMethodName() = ["gsub", "gsub!"] and effectiveGsubRegExp(re)
+        )
+      )
+    }
+  }
+
+  /**
+   * A node sanitized by a prior call to `sub!` or `gsub!`,
+   * e.g. the `password` argument to `info` in:
+   * ```
+   * password = "changeme"
+   * password.sub!(/.+/, "")
+   * Logger.new(STDOUT).info password
+   * ```
+   */
+  private class MaskingReplacerSanitizedNode extends SanitizerIn {
+    MaskingReplacerSanitizedNode() {
+      exists(MaskingReplacerSanitizer maskCall, Variable v |
+        maskCall.getMethodName() = ["sub!", "gsub!"] and
+        v = maskCall.getReceiver().asExpr().getExpr().(VariableReadAccess).getVariable() and
+        v = this.asExpr().getExpr().(VariableReadAccess).getVariable() and
+        maskCall.asExpr().getASuccessor*() = this.asExpr()
+      )
+    }
+  }
+
+  /**
+   * Gets the name of a  method that would be falsely marked as non-sensitive
+   * by `notSensitiveRegexp`.
+   */
+  private predicate nonSensitiveMethodNameExclusion(string name) {
+    name = ["[]", "[]="]
+  }
+
+  /**
+   * A call that might obfuscate a password, for example through hashing.
+   */
+  private class ObfuscatorCall extends Sanitizer, DataFlow::CallNode {
+    ObfuscatorCall() {
+      this.getMethodName().regexpMatch(notSensitiveRegexp()) and
+      not nonSensitiveMethodNameExclusion(this.getMethodName())
+    }
+  }
+
+  /**
+   * A data flow node that does not contain a clear-text password, according to its syntactic name.
+   */
+  private class NameGuidedNonCleartextPassword extends NonCleartextPassword {
+    NameGuidedNonCleartextPassword() {
+      exists(string name | name.regexpMatch(notSensitiveRegexp()) |
+        // accessing a non-sensitive variable
+        this.asExpr().getExpr().(VariableReadAccess).getVariable().getName() = name
+        or
+        // dereferencing a non-sensitive field
+        this.asExpr()
+            .getExpr()
+            .(ElementReference)
+            .getArgument(0)
+            .getConstantValue()
+            .getStringOrSymbol() = name
+        or
+        // calling a non-sensitive method
+        (
+          this.(DataFlow::CallNode).getMethodName() = name and
+          not nonSensitiveMethodNameExclusion(name)
+        )
+      )
+      or
+      // avoid i18n strings
+      this.asExpr()
+          .getExpr()
+          .(ElementReference)
+          .getReceiver()
+          .getConstantValue()
+          .getStringOrSymbol()
+          .regexpMatch("(?is).*(messages|strings).*")
+    }
+  }
+
+  /**
+   * A data flow node that receives flow that is not a clear-text password.
+   */
+  private class NonCleartextPasswordFlow extends NonCleartextPassword {
+    NonCleartextPasswordFlow() {
+      any(NonCleartextPassword other).(DataFlow::LocalSourceNode).flowsTo(this)
+    }
+  }
+
+  /**
+   * A data flow node that does not contain a clear-text password.
+   */
+  abstract private class NonCleartextPassword extends DataFlow::Node { }
+
+  // `writeNode` assigns pair with key `name` to `val`
+  private predicate hashKeyWrite(DataFlow::CallNode writeNode, string name, DataFlow::Node val) {
+    exists(SetterMethodCall setter |
+      setter = writeNode.asExpr().getExpr() and
+      // hash[name]
+      setter.getArgument(0).getConstantValue().getStringOrSymbol() = name and
+      // val
+      setter.getArgument(1).(Assignment).getRightOperand() = val.asExpr().getExpr()
+    )
+  }
+
+  /**
+   * An hash with a value that may contain password information
+   *
+   * This is a source since logging a hash will show the pairs present.
+   */
+  private abstract class HashPasswordSource extends Source {
+    /** Gets the name of the key */
+    abstract string getName();
+
+    /**
+     * Gets the name of the hash variable that this password source is assigned
+     * to, if applicable.
+     */
+    abstract LocalVariable getVariable();
+  }
+
+  private class HashKeyWritePasswordSource extends HashPasswordSource {
+    private string name;
+    private DataFlow::ExprNode recv;
+
+    HashKeyWritePasswordSource() {
+      exists(DataFlow::Node val |
+        name.regexpMatch(maybePassword()) and
+        not name.regexpMatch(notSensitiveRegexp()) and
+        // avoid safe values assigned to presumably unsafe names
+        not val instanceof NonCleartextPassword and
+        (
+          // hash[name] = val
+          hashKeyWrite(this, name, val) and
+          recv = this.(DataFlow::CallNode).getReceiver()
+        )
+      )
+    }
+
+    override string describe() { result = "an write to " + name }
+    override string getName() { result = name }
+    override LocalVariable getVariable() {
+      result = recv.getExprNode().getExpr().(VariableReadAccess).getVariable()
+    }
+  }
+
+  private class HashLiteralPasswordSource extends HashPasswordSource {
+    private string name;
+    private HashLiteral lit;
+
+    HashLiteralPasswordSource() {
+      exists(DataFlow::Node val |
+        name.regexpMatch(maybePassword()) and
+        not name.regexpMatch(notSensitiveRegexp()) and
+        // avoid safe values assigned to presumably unsafe names
+        not val instanceof NonCleartextPassword and
+        // hash = { name: val }
+        exists(Pair p |
+          this.asExpr().getExpr() = lit and p = lit.getAKeyValuePair() |
+          p.getKey().getConstantValue().getStringOrSymbol() = name and
+          p.getValue() = val.asExpr().getExpr()
+        )
+      )
+    }
+
+    override string describe() { result = "an write to " + name }
+    override string getName() { result = name }
+    override LocalVariable getVariable() {
+      exists(Assignment a |
+        a.getRightOperand() = lit |
+        result = a.getLeftOperand().getAVariable()
+      )
+    }
+  }
+
+  /** An assignment that may assign a password to a variable */
+  private class AssignPasswordVariableSource extends Source {
+    string name;
+
+    AssignPasswordVariableSource() {
+      // avoid safe values assigned to presumably unsafe names
+      not this instanceof NonCleartextPassword and
+      name.regexpMatch(maybePassword()) and
+      (
+        exists(Assignment a |
+          this.asExpr().getExpr() = a.getRightOperand() and
+          a.getLeftOperand().getAVariable().getName() = name)
+      )
+    }
+
+    override string describe() { result = "an assignment to " + name }
+  }
+
+  /** A parameter that may contain a password. */
+  private class ParameterPasswordSource extends Source {
+    private string name;
+
+    ParameterPasswordSource() {
+      name.regexpMatch(maybePassword()) and
+      not this instanceof NonCleartextPassword and
+      exists(Parameter p, LocalVariable v |
+        v = p.getAVariable() and
+        v.getName() = name and
+        this.asExpr().getExpr() = v.getAnAccess()
+      )
+    }
+
+    override string describe() { result = "a parameter " + name }
+  }
+
+  /** A call that might return a password. */
+  private class CallPasswordSource extends DataFlow::CallNode, Source {
+    private string name;
+
+    CallPasswordSource() {
+      name = this.getMethodName() and
+      name.regexpMatch("(?is)getPassword")
+    }
+
+    override string describe() { result = "a call to " + name }
+  }
+
+  private string commonLogMethodName() {
+    result = ["info", "debug", "warn", "warning", "error", "log"]
+  }
+
+  /** Holds if `nodeFrom` taints `nodeTo`. */
+  predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    exists(string name, ElementReference ref, LocalVariable hashVar |
+      // from `hsh[password] = "changeme"` to a `hsh[password]` read
+      nodeFrom.(HashKeyWritePasswordSource).getName() = name and
+      nodeTo.asExpr().getExpr() = ref and
+      ref.getArgument(0).getConstantValue().getStringOrSymbol() = name and
+      nodeFrom.(HashPasswordSource).getVariable() = hashVar and
+      ref.getReceiver().(VariableReadAccess).getVariable() = hashVar and
+      nodeFrom.asExpr().getASuccessor*() = nodeTo.asExpr()
+    )
+  }
+
+  /**
+   * A node representing an expression whose value is logged.
+   */
+  private class LoggingInputAsSink extends Sink {
+    LoggingInputAsSink() {
+      // precise match based on inferred type of receiver
+      exists(Logging logging | this = logging.getAnInput())
+      or
+      // imprecise name based match
+      exists(DataFlow::CallNode call, string recvName |
+        recvName =
+          call.getReceiver().asExpr().getExpr().(VariableReadAccess).getVariable().getName() and
+        recvName.regexpMatch(".*log(ger)?") and
+        call.getMethodName() = commonLogMethodName()
+      |
+        this = call.getArgument(_)
+      )
+    }
+  }
+}

ruby/ql/lib/codeql/ruby/security/CleartextLoggingQuery.qll

@@ -0,0 +1,38 @@
+/**
+ * Provides a taint-tracking configuration for "Clear-text logging of sensitive information".
+ *
+ * Note, for performance reasons: only import this file if
+ * `CleartextLogging::Configuration` is needed, otherwise
+ * `CleartextLoggingCustomizations` should be imported instead.
+ */
+
+private import ruby
+private import codeql.ruby.DataFlow
+private import codeql.ruby.TaintTracking
+import CleartextLoggingCustomizations::CleartextLogging
+private import CleartextLoggingCustomizations::CleartextLogging as CleartextLogging
+
+/**
+ * A taint-tracking configuration for detecting "Clear-text logging of sensitive information".
+ */
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "CleartextLogging" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof CleartextLogging::Source }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof CleartextLogging::Sink }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    super.isSanitizer(node)
+    or
+    node instanceof CleartextLogging::Sanitizer
+  }
+
+  override predicate isSanitizerIn(DataFlow::Node node) {
+    node instanceof CleartextLogging::SanitizerIn
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
+    CleartextLogging::isAdditionalTaintStep(nodeFrom, nodeTo)
+  }
+}

ruby/ql/src/queries/security/cwe-312/CleartextLogging.ql

@@ -0,0 +1,24 @@
+/**
+ * @name Clear-text logging of sensitive information
+ * @description Logging sensitive information without encryption or hashing can
+ *              expose it to an attacker.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 7.5
+ * @precision high
+ * @id rb/clear-text-logging-sensitive-data
+ * @tags security
+ *       external/cwe/cwe-312
+ *       external/cwe/cwe-359
+ *       external/cwe/cwe-532
+ */
+
+import ruby
+import codeql.ruby.security.CleartextLoggingQuery
+import codeql.ruby.DataFlow
+import DataFlow::PathGraph
+
+from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink.getNode(), source, sink,
+  "Sensitive data returned by $@ is logged here.", source.getNode(), source.getNode().(Source).describe()

ruby/ql/test/query-tests/security/cwe-312/CleartextLogging.expected

@@ -0,0 +1,4 @@
+edges
+nodes
+subpaths
+#select

ruby/ql/test/query-tests/security/cwe-312/CleartextLogging.qlref

@@ -0,0 +1 @@
+queries/security/cwe-312/CleartextLogging.ql

ruby/ql/test/query-tests/security/cwe-312/logging.rb

@@ -0,0 +1,84 @@
+stdout_logger = Logger.new STDOUT
+
+password = "043697b96909e03ca907599d6420555f"
+
+# BAD: password logged as plaintext
+stdout_logger.info password
+# BAD: password logged as plaintext
+stdout_logger.debug password
+# BAD: password logged as plaintext
+stdout_logger.error password
+# BAD: password logged as plaintext
+stdout_logger.fatal password
+# BAD: password logged as plaintext
+stdout_logger.unknown password
+# BAD: password logged as plaintext
+stdout_logger.warn password
+
+# BAD: password logged as plaintext
+stdout_logger.add Logger::WARN, password
+# BAD: password logged as plaintext
+stdout_logger.add Logger::WARN, "message", password
+# BAD: password logged as plaintext
+stdout_logger.log Logger::WARN, password
+
+# BAD: password logged as plaintext
+stdout_logger << "pw: #{password}"
+# BAD: sensitive data in the progname will taint subsequent logging calls
+stdout_logger.progname = password
+
+hsh1 = { password: "aec5058e61f7f122998b1a30ee2c66b6" }
+hsh2 = {}
+# GOOD: no backwards flow
+stdout_logger.info hsh2[:password]
+hsh2[:password] = "beeda625d7306b45784d91ea0336e201"
+
+# BAD: password logged as plaintext
+stdout_logger.info hsh1[:password]
+# BAD: password logged as plaintext
+stdout_logger.info hsh2[:password]
+
+password_masked_sub = "ca497451f5e883662fb1a37bc9ec7838"
+password_masked_sub_ex = "ca497451f5e883662fb1a37bc9ec7838"
+password_masked_gsub = "a7e3747b19930d4f4b8181047194832f"
+password_masked_gsub_ex = "a7e3747b19930d4f4b8181047194832f"
+password_masked_sub = password_masked_sub.sub(/.+/, "[password]")
+password_masked_sub_ex.sub!(/.+/, "[password]")
+password_masked_gsub = password_masked_gsub.gsub(/./, "*")
+password_masked_gsub_ex.gsub!(/./, "*")
+
+# GOOD: password is effectively masked before logging
+stdout_logger.info password_masked_sub
+# GOOD: password is effectively masked before logging
+stdout_logger.info password_masked_gsub
+# GOOD: password is effectively masked before logging
+stdout_logger.info password_masked_sub_ex
+# GOOD: password is effectively masked before logging
+stdout_logger.info password_masked_gsub_ex
+
+password_masked_ineffective_sub = "ca497451f5e883662fb1a37bc9ec7838"
+password_masked_ineffective_sub_ex = "ca497451f5e883662fb1a37bc9ec7838"
+password_masked_ineffective_gsub = "a7e3747b19930d4f4b8181047194832f"
+password_masked_ineffective_gsub_ex = "a7e3747b19930d4f4b8181047194832f"
+password_masked_ineffective_sub = password_masked_ineffective_sub.sub(/./, "[password]")
+password_masked_ineffective_sub_ex.sub!(/./, "[password]")
+password_masked_ineffective_gsub = password_masked_ineffective_gsub.gsub(/[A-Z]/, "*")
+password_masked_ineffective_gsub_ex.gsub!(/[A-Z]/, "*")
+
+# BAD: password masked ineffectively
+stdout_logger.info password_masked_ineffective_sub
+# BAD: password masked ineffectively
+stdout_logger.info password_masked_ineffective_gsub
+# BAD: password masked ineffectively
+stdout_logger.info password_masked_ineffective_sub_ex
+# BAD: password masked ineffectively
+stdout_logger.info password_masked_ineffective_gsub_ex
+
+def foo(password, logger)
+  # BAD: password logged as plaintext
+  logger.info password
+end
+
+password_arg = "65f2950df2f0e2c38d7ba2ccca767291"
+foo(password_arg, stdout_logger)
+foo("65f2950df2f0e2c38d7ba2ccca767292", stdout_logger)