WIP: HardcodedCredentials query

ql/src/queries/security/cwe-798/Debug.ql

@@ -0,0 +1,129 @@
+/*
+ * @name Hard-coded credentials
+ * @description Credentials are hard coded in the source code of the application.
+ * @problem.severity error
+ * @precision medium TODO
+ * @id rb/hardcoded-credentials
+ * @tags security
+ *       external/cwe/cwe-259
+ *       external/cwe/cwe-321
+ *       external/cwe/cwe-798
+ */
+
+import ruby
+import codeql_ruby.DataFlow
+import DataFlow::PathGraph
+private import codeql_ruby.dataflow.SSA
+private import codeql_ruby.CFG
+
+bindingset[char, fraction]
+predicate fewer_characters_than(StringLiteral str, string char, float fraction) {
+  exists(string text, int chars |
+    text = str.getValueText() and
+    chars = count(int i | text.charAt(i) = char)
+  |
+    /* Allow one character */
+    chars = 1 or
+    chars < text.length() * fraction
+  )
+}
+
+predicate possible_reflective_name(string name) {
+  // TODO: implement me
+  none()
+}
+
+int char_count(StringLiteral str) { result = count(string c | c = str.getValueText().charAt(_)) }
+
+predicate capitalized_word(StringLiteral str) { str.getValueText().regexpMatch("[A-Z][a-z]+") }
+
+predicate format_string(StringLiteral str) { str.getValueText().matches("%{%}%") }
+
+predicate maybeCredential(Expr e) {
+  /* A string that is not too short and unlikely to be text or an identifier. */
+  exists(StringLiteral str | str = e |
+    /* At least 10 characters */
+    str.getValueText().length() > 9 and
+    /* Not too much whitespace */
+    fewer_characters_than(str, " ", 0.05) and
+    /* or underscores */
+    fewer_characters_than(str, "_", 0.2) and
+    /* Not too repetitive */
+    exists(int chars | chars = char_count(str) |
+      chars > 15 or
+      chars * 3 > str.getValueText().length() * 2
+    ) and
+    not possible_reflective_name(str.getValueText()) and
+    not capitalized_word(str) and
+    not format_string(str)
+  )
+  or
+  /* Or, an integer with over 32 bits */
+  exists(IntegerLiteral lit | lit = e |
+    not exists(lit.getValue()) and
+    /* Not a set of flags or round number */
+    not lit.getValueText().matches("%00%")
+  )
+}
+
+class HardcodedValueSource extends DataFlow::Node {
+  HardcodedValueSource() { maybeCredential(this.asExpr().getExpr()) }
+}
+
+/**
+ * Gets a regular expression for matching names of locations (variables, parameters, keys) that
+ * indicate the value being held is a credential.
+ */
+private string getACredentialRegex() {
+  result = "(?i).*pass(wd|word|code|phrase)(?!.*question).*" or
+  result = "(?i).*(puid|username|userid).*" or
+  result = "(?i).*(cert)(?!.*(format|name)).*"
+}
+
+private predicate isCredentialSink(Expr e) {
+  exists(string name |
+    name.regexpMatch(getACredentialRegex()) and
+    not name.suffix(name.length() - 4) = "file"
+  |
+    // A method call with a parameter that may hold a credential
+    exists(Method m, NamedParameter p, int idx, MethodCall mc |
+      mc.getArgument(idx).getAChild*() = e and
+      p = m.getParameter(idx) and
+      p.getName() = name and
+      // TODO: link call w/ method more precisely
+      mc.getMethodName() = m.getName()
+    )
+    or
+    // An equality check against a credential value
+    exists(EqualityOperation op, VariableReadAccess vra | vra.getVariable().getName() = name |
+      op.getLeftOperand() = e and op.getRightOperand() = vra
+      or
+      op.getLeftOperand() = vra and op.getRightOperand() = e
+    )
+    /*
+     *      or
+     *      exists(Keyword k | k.getArg() = name and k.getValue().getAFlowNode() = this)
+     */
+
+    )
+}
+
+class CredentialSink extends DataFlow::Node {
+  CredentialSink() { isCredentialSink(this.asExpr().getExpr()) }
+}
+
+class HardcodedCredentialsConfiguration extends DataFlow::Configuration {
+  HardcodedCredentialsConfiguration() { this = "HardcodedCredentialsConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof HardcodedValueSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof CredentialSink }
+}
+
+from Method m, NamedParameter p, int idx, MethodCall mc, Expr e
+where
+  mc.getArgument(idx) = e and
+  p = m.getParameter(idx) and
+  // TODO: link call w/ method more precisely
+  mc.getMethodName() = m.getName()
+select m, p, idx, mc, e

ql/src/queries/security/cwe-798/HardcodedCredentials.qhelp

@@ -0,0 +1,79 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Including unencrypted hard-coded inbound or outbound authentication credentials within source code
+or configuration files is dangerous because the credentials may be easily discovered.
+</p>
+<p>
+Source or configuration files containing hard-coded credentials may be visible to an attacker. For
+example, the source code may be open source, or it may be leaked or accidentally revealed.
+</p>
+<p>
+For inbound authentication, hard-coded credentials may allow unauthorized access to the system. This
+is particularly problematic if the credential is hard-coded in the source code, because it cannot be
+disabled easily. For outbound authentication, the hard-coded credentials may provide an attacker with
+privileged information or unauthorized access to some other system.
+</p>
+
+</overview>
+<recommendation>
+
+<p>
+Remove hard-coded credentials, such as user names, passwords and certificates, from source code,
+placing them in configuration files or other data stores if necessary. If possible, store
+configuration files including credential data separately from the source code, in a secure location
+with restricted access.
+</p>
+
+<p>
+For outbound authentication details, consider encrypting the credentials or the enclosing data
+stores or configuration files, and using permissions to restrict access.
+</p>
+
+<p>
+For inbound authentication details, consider hashing passwords using standard library functions
+where possible. For example, <code>OpenSSL::KDF.pbkdf2_hmac</code>.
+</p>
+
+</recommendation>
+<example>
+
+<p>
+The following examples shows different types of inbound and outbound authentication.
+</p>
+
+<p>
+In the first case, <code>RackAppBad</code>, we accept a password from a remote user, and compare
+it against a plaintext string literal. If an attacker acquires the source code they can observe
+the password, and can log in to the system. Furthermore, if such an intrusion was discovered, the
+application would need to be rewritten and redeployed in order to change the password.
+</p>
+
+<p>
+In the second case, <code>RackAppGood</code>, the password is compared to a hashed and salted
+password stored in a configuration file, using <code>OpenSSL::KDF.pbkdf2_hmac</code>.
+In this case, access to the source code or the assembly would not reveal the password to an
+attacker. Even access to the configuration file containing the password hash and salt would be of
+little value to an attacker, as it is usually extremely difficult to reverse engineer the password
+from the hash and salt. In a real application care should be taken to make the string comparison
+of the hashed input against the hashed password take close to constant time, as this will make
+timing attacks more difficult.
+</p>
+
+<sample src="HardcodedCredentials.rb" />
+
+</example>
+<references>
+
+<li>
+OWASP:
+<a href="https://www.owasp.org/index.php/Use_of_hard-coded_password">XSS
+Use of hard-coded password</a>.
+</li>
+
+</references>
+</qhelp>

ql/src/queries/security/cwe-798/HardcodedCredentials.ql

@@ -0,0 +1,124 @@
+/*
+ * @name Hard-coded credentials
+ * @description Credentials are hard coded in the source code of the application.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision medium
+ * @id rb/hardcoded-credentials
+ * @tags security
+ *       external/cwe/cwe-259
+ *       external/cwe/cwe-321
+ *       external/cwe/cwe-798
+ */
+
+// TODO: adjust precision
+
+import ruby
+import codeql_ruby.DataFlow
+import DataFlow::PathGraph
+private import codeql_ruby.dataflow.SSA
+private import codeql_ruby.CFG
+
+bindingset[char, fraction]
+predicate fewer_characters_than(StringLiteral str, string char, float fraction) {
+  exists(string text, int chars |
+    text = str.getValueText() and
+    chars = count(int i | text.charAt(i) = char)
+  |
+    /* Allow one character */
+    chars = 1 or
+    chars < text.length() * fraction
+  )
+}
+
+predicate possible_reflective_name(string name) {
+  // TODO: implement this?
+  none()
+}
+
+int char_count(StringLiteral str) { result = count(string c | c = str.getValueText().charAt(_)) }
+
+predicate capitalized_word(StringLiteral str) { str.getValueText().regexpMatch("[A-Z][a-z]+") }
+
+predicate format_string(StringLiteral str) { str.getValueText().matches("%{%}%") }
+
+predicate maybeCredential(Expr e) {
+  /* A string that is not too short and unlikely to be text or an identifier. */
+  exists(StringLiteral str | str = e |
+    /* At least 10 characters */
+    str.getValueText().length() > 9 and
+    /* Not too much whitespace */
+    fewer_characters_than(str, " ", 0.05) and
+    /* or underscores */
+    fewer_characters_than(str, "_", 0.2) and
+    /* Not too repetitive */
+    exists(int chars | chars = char_count(str) |
+      chars > 15 or
+      chars * 3 > str.getValueText().length() * 2
+    ) and
+    not possible_reflective_name(str.getValueText()) and
+    not capitalized_word(str) and
+    not format_string(str)
+  )
+  or
+  /* Or, an integer with over 32 bits */
+  exists(IntegerLiteral lit | lit = e |
+    not exists(lit.getValue()) and
+    /* Not a set of flags or round number */
+    not lit.getValueText().matches("%00%")
+  )
+}
+
+class HardcodedValueSource extends DataFlow::Node {
+  HardcodedValueSource() { maybeCredential(this.asExpr().getExpr()) }
+}
+
+/**
+ * Gets a regular expression for matching names of locations (variables, parameters, keys) that
+ * indicate the value being held is a credential.
+ */
+private string getACredentialRegex() {
+  result = "(?i).*pass(wd|word|code|phrase)(?!.*question).*" or
+  result = "(?i).*(puid|username|userid).*" or
+  result = "(?i).*(cert)(?!.*(format|name)).*"
+}
+
+private predicate isCredentialSink(Expr e) {
+  exists(string name |
+    name.regexpMatch(getACredentialRegex()) and
+    not name.suffix(name.length() - 4) = "file"
+  |
+    // A method call with a parameter that may hold a credential
+    exists(Method m, NamedParameter p, int idx, MethodCall mc |
+      // Include keyword argument values etc.
+      mc.getArgument(idx).getAChild*() = e and
+      p = m.getParameter(idx) and
+      p.getName() = name and
+      // TODO: link call w/ method more precisely
+      mc.getMethodName() = m.getName()
+    )
+    or
+    // An equality check against a credential value
+    exists(EqualityOperation op, VariableReadAccess vra | vra.getVariable().getName() = name |
+      op.getLeftOperand() = e and op.getRightOperand() = vra
+      or
+      op.getLeftOperand() = vra and op.getRightOperand() = e
+    )
+  )
+}
+
+class CredentialSink extends DataFlow::Node {
+  CredentialSink() { isCredentialSink(this.asExpr().getExpr()) }
+}
+
+class HardcodedCredentialsConfiguration extends DataFlow::Configuration {
+  HardcodedCredentialsConfiguration() { this = "HardcodedCredentialsConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof HardcodedValueSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof CredentialSink }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, HardcodedCredentialsConfiguration conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Use of $@.", source.getNode(), "hardcoded credentials"

ql/src/queries/security/cwe-798/HardcodedCredentials.rb

@@ -0,0 +1,40 @@
+require 'rack'
+require 'yaml'
+require 'openssl'
+
+class RackAppBad
+  def call(env)
+    req = Rack::Request.new(env)
+    password = req.params['password']
+
+    # BAD: Inbound authentication made by comparison to string literal
+    if password == 'myPa55word'
+      [200, {'Content-type' => 'text/plain'}, ['OK']]
+    else
+      [403, {'Content-type' => 'text/plain'}, ['Permission denied']]
+    end
+  end
+end
+
+class RackAppGood
+  def call(env)
+    req = Rack::Request.new(env)
+    password = req.params['password']
+
+    config_file = YAML.load_file('config.yml')
+    hashed_password = config_file['hashed_password']
+    salt = [config_file['salt']].pack('H*')
+
+    #GOOD: Inbound authentication made by comparing to a hash password from a config file.
+    hash = OpenSSL::Digest::SHA256.new
+    dk = OpenSSL::KDF.pbkdf2_hmac(
+      password, salt: salt, hash: hash, iterations: 100_000, length: hash.digest_length
+    )
+    hashed_input = dk.unpack('H*').first
+    if hashed_password == hashed_input
+      [200, {'Content-type' => 'text/plain'}, ['OK']]
+    else
+      [403, {'Content-type' => 'text/plain'}, ['Permission denied']]
+    end
+  end
+end

ql/src/queries/security/cwe-798/Modules.ql

@@ -0,0 +1,21 @@
+import ruby
+
+/*
+ * predicate possible_reflective_name(string name) {
+ *  exists(any(ModuleValue m).attr(name))
+ *  or
+ *  exists(any(ClassValue c).lookup(name))
+ *  or
+ *  any(ClassValue c).getName() = name
+ *  or
+ *  exists(Module::named(name))
+ *  or
+ *  exists(Value::named(name))
+ * }
+ */
+
+string module_name() { result = any(Namespace m | | m.getName()) }
+
+from string s
+where s = module_name()
+select s

ql/src/queries/security/cwe-798/config.yml

@@ -0,0 +1,2 @@
+salt: "5a3d24eb243ac91e86ff2d06589b2c04"
+hashed_password: "c5c19cdc336d6877a76a7a2aeaac142564f8998fd168f7f741192d268e2f94c1"

ql/test/query-tests/security/cwe-798/Debug.qlref

@@ -0,0 +1 @@
+queries/security/cwe-798/Debug.ql

ql/test/query-tests/security/cwe-798/HardcodedCredentials.expected

@@ -0,0 +1,13 @@
+edges
+nodes
+| HardcodedCredentials.rb:4:20:4:65 | "xwjVWdfzfRlbcgKkbSfG/xSrUeHYq..." | semmle.label | "xwjVWdfzfRlbcgKkbSfG/xSrUeHYq..." |
+| HardcodedCredentials.rb:8:30:8:75 | "X6BLgRWSAtAWG/GaHS+WGGW2K7zZF..." | semmle.label | "X6BLgRWSAtAWG/GaHS+WGGW2K7zZF..." |
+| HardcodedCredentials.rb:12:19:12:64 | "4NQX/CqB5Ae98zFUmwj1DMpF7azsh..." | semmle.label | "4NQX/CqB5Ae98zFUmwj1DMpF7azsh..." |
+| HardcodedCredentials.rb:15:30:15:75 | "WLC17dLQ9P8YlQvqm77qplOMm5pd1..." | semmle.label | "WLC17dLQ9P8YlQvqm77qplOMm5pd1..." |
+| HardcodedCredentials.rb:18:27:18:72 | "4NQX/CqB5Ae98zFUmwj1DMpF7azsh..." | semmle.label | "4NQX/CqB5Ae98zFUmwj1DMpF7azsh..." |
+#select
+| HardcodedCredentials.rb:4:20:4:65 | "xwjVWdfzfRlbcgKkbSfG/xSrUeHYq..." | HardcodedCredentials.rb:4:20:4:65 | "xwjVWdfzfRlbcgKkbSfG/xSrUeHYq..." | HardcodedCredentials.rb:4:20:4:65 | "xwjVWdfzfRlbcgKkbSfG/xSrUeHYq..." | Use of $@. | HardcodedCredentials.rb:4:20:4:65 | "xwjVWdfzfRlbcgKkbSfG/xSrUeHYq..." | hardcoded credentials |
+| HardcodedCredentials.rb:8:30:8:75 | "X6BLgRWSAtAWG/GaHS+WGGW2K7zZF..." | HardcodedCredentials.rb:8:30:8:75 | "X6BLgRWSAtAWG/GaHS+WGGW2K7zZF..." | HardcodedCredentials.rb:8:30:8:75 | "X6BLgRWSAtAWG/GaHS+WGGW2K7zZF..." | Use of $@. | HardcodedCredentials.rb:8:30:8:75 | "X6BLgRWSAtAWG/GaHS+WGGW2K7zZF..." | hardcoded credentials |
+| HardcodedCredentials.rb:12:19:12:64 | "4NQX/CqB5Ae98zFUmwj1DMpF7azsh..." | HardcodedCredentials.rb:12:19:12:64 | "4NQX/CqB5Ae98zFUmwj1DMpF7azsh..." | HardcodedCredentials.rb:12:19:12:64 | "4NQX/CqB5Ae98zFUmwj1DMpF7azsh..." | Use of $@. | HardcodedCredentials.rb:12:19:12:64 | "4NQX/CqB5Ae98zFUmwj1DMpF7azsh..." | hardcoded credentials |
+| HardcodedCredentials.rb:15:30:15:75 | "WLC17dLQ9P8YlQvqm77qplOMm5pd1..." | HardcodedCredentials.rb:15:30:15:75 | "WLC17dLQ9P8YlQvqm77qplOMm5pd1..." | HardcodedCredentials.rb:15:30:15:75 | "WLC17dLQ9P8YlQvqm77qplOMm5pd1..." | Use of $@. | HardcodedCredentials.rb:15:30:15:75 | "WLC17dLQ9P8YlQvqm77qplOMm5pd1..." | hardcoded credentials |
+| HardcodedCredentials.rb:18:27:18:72 | "4NQX/CqB5Ae98zFUmwj1DMpF7azsh..." | HardcodedCredentials.rb:18:27:18:72 | "4NQX/CqB5Ae98zFUmwj1DMpF7azsh..." | HardcodedCredentials.rb:18:27:18:72 | "4NQX/CqB5Ae98zFUmwj1DMpF7azsh..." | Use of $@. | HardcodedCredentials.rb:18:27:18:72 | "4NQX/CqB5Ae98zFUmwj1DMpF7azsh..." | hardcoded credentials |

ql/test/query-tests/security/cwe-798/HardcodedCredentials.qlref

@@ -0,0 +1 @@
+queries/security/cwe-798/HardcodedCredentials.ql

ql/test/query-tests/security/cwe-798/HardcodedCredentials.rb

@@ -0,0 +1,22 @@
+def authenticate(uid, password, cert: nil)
+  if cert != nil then
+    # comparison with hardcoded credential
+    return cert == "xwjVWdfzfRlbcgKkbSfG/xSrUeHYqxPgz9WKN3Yow1o="
+  end
+
+  # comparison with hardcoded credential
+  uid == 123 and password == "X6BLgRWSAtAWG/GaHS+WGGW2K7zZFTAjJ54fGSudHJk="
+end
+
+# call with hardcoded credential as argument
+authenticate(123, "4NQX/CqB5Ae98zFUmwj1DMpF7azshxSvb0Jo4gIFmIQ=")
+
+# call with hardcoded credential as argument
+authenticate(456, nil, cert: "WLC17dLQ9P8YlQvqm77qplOMm5pd1q25Q2onWqu78JI=")
+
+# concatenation involving literal
+authenticate(789, "pw:" + "4NQX/CqB5Ae98zFUmwj1DMpF7azshxSvb0Jo4gIFmIQ=")
+
+passwd = gets.chomp
+# call with hardcoded credential-like value, but not to a potential credential sink (should not be flagged)
+authenticate("gowLsSGfPbh/ZS60k+LQQBhcq1tsh/YgbvNmDauQr5Q=", passwd)