mirror of
https://github.com/github/codeql.git
synced 2026-04-24 08:15:14 +02:00
Merge pull request #187 from github/hardcoded-credentials
Add rb/hardcoded-credentials query
This commit is contained in:
Alex Ford
79
ql/src/queries/security/cwe-798/HardcodedCredentials.qhelp
Normal file
79
ql/src/queries/security/cwe-798/HardcodedCredentials.qhelp
Normal file
@@ -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>
|
||||
153
ql/src/queries/security/cwe-798/HardcodedCredentials.ql
Normal file
153
ql/src/queries/security/cwe-798/HardcodedCredentials.ql
Normal file
@@ -0,0 +1,153 @@
|
||||
/*
|
||||
* @name Hard-coded credentials
|
||||
* @description Credentials are hard coded in the source code of the application.
|
||||
* @kind path-problem
|
||||
* @problem.severity error
|
||||
* @precision high
|
||||
* @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.controlflow.CfgNodes
|
||||
|
||||
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)).*"
|
||||
}
|
||||
|
||||
bindingset[name]
|
||||
private predicate maybeCredentialName(string name) {
|
||||
name.regexpMatch(getACredentialRegex()) and
|
||||
not name.suffix(name.length() - 4) = "file"
|
||||
}
|
||||
|
||||
// Positional parameter
|
||||
private DataFlow::Node credentialParameter() {
|
||||
exists(Method m, NamedParameter p, int idx |
|
||||
result.asParameter() = p and
|
||||
p = m.getParameter(idx) and
|
||||
maybeCredentialName(p.getName())
|
||||
)
|
||||
}
|
||||
|
||||
// Keyword argument
|
||||
private Expr credentialKeywordArgument() {
|
||||
exists(MethodCall mc, string argKey |
|
||||
result = mc.getKeywordArgument(argKey) and
|
||||
maybeCredentialName(argKey)
|
||||
)
|
||||
}
|
||||
|
||||
// An equality check against a credential value
|
||||
private Expr credentialComparison() {
|
||||
exists(EqualityOperation op, VariableReadAccess vra |
|
||||
maybeCredentialName(vra.getVariable().getName()) and
|
||||
(
|
||||
op.getLeftOperand() = result and
|
||||
op.getRightOperand() = vra
|
||||
or
|
||||
op.getLeftOperand() = vra and op.getRightOperand() = result
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
private predicate isCredentialSink(DataFlow::Node node) {
|
||||
node = credentialParameter()
|
||||
or
|
||||
node.asExpr().getExpr() = credentialKeywordArgument()
|
||||
or
|
||||
node.asExpr().getExpr() = credentialComparison()
|
||||
}
|
||||
|
||||
class CredentialSink extends DataFlow::Node {
|
||||
CredentialSink() { isCredentialSink(this) }
|
||||
}
|
||||
|
||||
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 }
|
||||
|
||||
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
|
||||
exists(ExprNodes::BinaryOperationCfgNode binop |
|
||||
(
|
||||
binop.getLeftOperand() = node1.asExpr() or
|
||||
binop.getRightOperand() = node1.asExpr()
|
||||
) and
|
||||
binop = node2.asExpr() and
|
||||
// string concatenation
|
||||
binop.getExpr() instanceof AddExpr
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
from DataFlow::PathNode source, DataFlow::PathNode sink, HardcodedCredentialsConfiguration conf
|
||||
where conf.hasFlowPath(source, sink)
|
||||
select source.getNode(), source, sink, "Use of $@.", source.getNode(), "hardcoded credentials"
|
||||
40
ql/src/queries/security/cwe-798/HardcodedCredentials.rb
Normal file
40
ql/src/queries/security/cwe-798/HardcodedCredentials.rb
Normal file
@@ -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
|
||||
@@ -0,0 +1,30 @@
|
||||
edges
|
||||
| HardcodedCredentials.rb:12:19:12:64 | "4NQX/CqB5Ae98zFUmwj1DMpF7azsh..." : | HardcodedCredentials.rb:1:23:1:30 | password |
|
||||
| HardcodedCredentials.rb:18:19:18:72 | ... + ... : | HardcodedCredentials.rb:1:23:1:30 | password |
|
||||
| HardcodedCredentials.rb:18:27:18:72 | "ogH6qSYWGdbR/2WOGYa7eZ/tObL+G..." : | HardcodedCredentials.rb:18:19:18:72 | ... + ... : |
|
||||
| HardcodedCredentials.rb:20:11:20:76 | "3jOe7sXKX6Tx52qHWUVqh2t9LNsE+..." : | HardcodedCredentials.rb:23:19:23:20 | pw : |
|
||||
| HardcodedCredentials.rb:21:12:21:37 | "4fQuzXef4f2yow8KWvIJTA==" : | HardcodedCredentials.rb:23:19:23:20 | pw : |
|
||||
| HardcodedCredentials.rb:23:19:23:20 | pw : | HardcodedCredentials.rb:1:23:1:30 | password |
|
||||
| HardcodedCredentials.rb:38:40:38:85 | "kdW/xVhiv6y1fQQNevDpUaq+2rfPK..." : | HardcodedCredentials.rb:31:18:31:23 | passwd |
|
||||
nodes
|
||||
| HardcodedCredentials.rb:1:23:1:30 | password | semmle.label | password |
|
||||
| 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:19:18:72 | ... + ... : | semmle.label | ... + ... : |
|
||||
| HardcodedCredentials.rb:18:27:18:72 | "ogH6qSYWGdbR/2WOGYa7eZ/tObL+G..." : | semmle.label | "ogH6qSYWGdbR/2WOGYa7eZ/tObL+G..." : |
|
||||
| HardcodedCredentials.rb:20:11:20:76 | "3jOe7sXKX6Tx52qHWUVqh2t9LNsE+..." : | semmle.label | "3jOe7sXKX6Tx52qHWUVqh2t9LNsE+..." : |
|
||||
| HardcodedCredentials.rb:21:12:21:37 | "4fQuzXef4f2yow8KWvIJTA==" : | semmle.label | "4fQuzXef4f2yow8KWvIJTA==" : |
|
||||
| HardcodedCredentials.rb:23:19:23:20 | pw : | semmle.label | pw : |
|
||||
| HardcodedCredentials.rb:31:18:31:23 | passwd | semmle.label | passwd |
|
||||
| HardcodedCredentials.rb:38:40:38:85 | "kdW/xVhiv6y1fQQNevDpUaq+2rfPK..." : | semmle.label | "kdW/xVhiv6y1fQQNevDpUaq+2rfPK..." : |
|
||||
#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:1:23:1:30 | password | 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 | "ogH6qSYWGdbR/2WOGYa7eZ/tObL+G..." | HardcodedCredentials.rb:18:27:18:72 | "ogH6qSYWGdbR/2WOGYa7eZ/tObL+G..." : | HardcodedCredentials.rb:1:23:1:30 | password | Use of $@. | HardcodedCredentials.rb:18:27:18:72 | "ogH6qSYWGdbR/2WOGYa7eZ/tObL+G..." | hardcoded credentials |
|
||||
| HardcodedCredentials.rb:20:11:20:76 | "3jOe7sXKX6Tx52qHWUVqh2t9LNsE+..." | HardcodedCredentials.rb:20:11:20:76 | "3jOe7sXKX6Tx52qHWUVqh2t9LNsE+..." : | HardcodedCredentials.rb:1:23:1:30 | password | Use of $@. | HardcodedCredentials.rb:20:11:20:76 | "3jOe7sXKX6Tx52qHWUVqh2t9LNsE+..." | hardcoded credentials |
|
||||
| HardcodedCredentials.rb:21:12:21:37 | "4fQuzXef4f2yow8KWvIJTA==" | HardcodedCredentials.rb:21:12:21:37 | "4fQuzXef4f2yow8KWvIJTA==" : | HardcodedCredentials.rb:1:23:1:30 | password | Use of $@. | HardcodedCredentials.rb:21:12:21:37 | "4fQuzXef4f2yow8KWvIJTA==" | hardcoded credentials |
|
||||
| HardcodedCredentials.rb:38:40:38:85 | "kdW/xVhiv6y1fQQNevDpUaq+2rfPK..." | HardcodedCredentials.rb:38:40:38:85 | "kdW/xVhiv6y1fQQNevDpUaq+2rfPK..." : | HardcodedCredentials.rb:31:18:31:23 | passwd | Use of $@. | HardcodedCredentials.rb:38:40:38:85 | "kdW/xVhiv6y1fQQNevDpUaq+2rfPK..." | hardcoded credentials |
|
||||
@@ -0,0 +1 @@
|
||||
queries/security/cwe-798/HardcodedCredentials.ql
|
||||
41
ql/test/query-tests/security/cwe-798/HardcodedCredentials.rb
Normal file
41
ql/test/query-tests/security/cwe-798/HardcodedCredentials.rb
Normal file
@@ -0,0 +1,41 @@
|
||||
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:" + "ogH6qSYWGdbR/2WOGYa7eZ/tObL+GtqDPx6q37BTTRQ=")
|
||||
|
||||
pw_left = "3jOe7sXKX6Tx52qHWUVqh2t9LNsE+ZXFj2qw6asRARTV2deAXFKkMTVOoaFYom1Q"
|
||||
pw_right = "4fQuzXef4f2yow8KWvIJTA=="
|
||||
pw = pw_left + pw_right
|
||||
authenticate(999, pw)
|
||||
|
||||
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)
|
||||
|
||||
module Passwords
|
||||
class KnownPasswords
|
||||
def include?(passwd)
|
||||
passwd == "foo"
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
# Call to object method
|
||||
Passwords::KnownPasswords.new.include?("kdW/xVhiv6y1fQQNevDpUaq+2rfPKfh+teE/45zS7bc=")
|
||||
|
||||
# Call to unrelated method with same name (should not be flagged)
|
||||
"foobar".include?("foo")
|
||||
Reference in New Issue
Block a user