diff --git a/change-notes/1.21/analysis-javascript.md b/change-notes/1.21/analysis-javascript.md index 8a63c8df65cc..c4af8bf7d716 100644 --- a/change-notes/1.21/analysis-javascript.md +++ b/change-notes/1.21/analysis-javascript.md @@ -40,7 +40,7 @@ | Expression has no effect | Fewer false-positive results | This rule now treats uses of `Object.defineProperty` more conservatively. | | Incomplete regular expression for hostnames | More results | This rule now tracks regular expressions for host names further. | | Incomplete string escaping or encoding | More results | This rule now considers the flow of regular expressions literals, and it no longer flags the removal of trailing newlines. | -| Password in configuration file | Fewer false positive results | This query now excludes passwords that are inserted into the configuration file using a templating mechanism or read from environment variables. | +| Password in configuration file | Fewer false positive results | This query now excludes passwords that are inserted into the configuration file using a templating mechanism or read from environment variables. Results are no longer shown on LGTM by default. | | Replacement of a substring with itself | More results | This rule now considers the flow of regular expressions literals. | | Server-side URL redirect | Fewer false-positive results | This rule now treats URLs as safe in more cases where the hostname cannot be tampered with. | | Type confusion through parameter tampering | Fewer false-positive results | This rule now recognizes additional emptiness checks. | diff --git a/javascript/ql/src/Security/CWE-313/PasswordInConfigurationFile.ql b/javascript/ql/src/Security/CWE-313/PasswordInConfigurationFile.ql index c62072b2f207..695f6fc29370 100644 --- a/javascript/ql/src/Security/CWE-313/PasswordInConfigurationFile.ql +++ b/javascript/ql/src/Security/CWE-313/PasswordInConfigurationFile.ql @@ -3,7 +3,7 @@ * @description Storing unencrypted passwords in configuration files is unsafe. * @kind problem * @problem.severity warning - * @precision high + * @precision medium * @id js/password-in-configuration-file * @tags security * external/cwe/cwe-256 @@ -12,6 +12,7 @@ */ import javascript +import semmle.javascript.RestrictedLocations /** * Holds if some JSON or YAML file contains a property with name `key` @@ -45,7 +46,7 @@ predicate exclude(File f) { f.getExtension().toLowerCase() = "raml" } -from string key, string val, Locatable valElement +from string key, string val, Locatable valElement, string pwd where config(key, val, valElement) and val != "" and @@ -53,13 +54,14 @@ where not val.regexpMatch(Templating::getDelimiterMatchingRegexp()) and ( key.toLowerCase() = "password" and + pwd = val and // exclude interpolations of environment variables not val.regexpMatch("\\$.*|%.*%") or key.toLowerCase() != "readme" and // look for `password=...`, but exclude `password=;`, `password="$(...)"`, // `password=%s` and `password==` - val.regexpMatch("(?is).*password\\s*=(?!\\s*;)(?!\"?[$`])(?!%s)(?!=).*") + pwd = val.regexpCapture("(?is).*password\\s*=\\s*(?!;|\"?[$`]|%s|=)(\\S+).*", 1) ) and not exclude(valElement.getFile()) -select valElement, "Avoid plaintext passwords in configuration files." +select (FirstLineOf)valElement, "Hard-coded password '" + pwd + "' in configuration file." diff --git a/javascript/ql/src/semmle/javascript/frameworks/Templating.qll b/javascript/ql/src/semmle/javascript/frameworks/Templating.qll index 08097dc46932..d9b45e1a421f 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Templating.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Templating.qll @@ -37,6 +37,6 @@ module Templating { * storing it in its first (and only) capture group. */ string getDelimiterMatchingRegexp() { - result = ".*(" + concat("\\Q" + getADelimiter() + "\\E", "|") + ").*" + result = "(?s).*(" + concat("\\Q" + getADelimiter() + "\\E", "|") + ").*" } } diff --git a/javascript/ql/test/query-tests/Security/CWE-313/PasswordInConfigurationFile.expected b/javascript/ql/test/query-tests/Security/CWE-313/PasswordInConfigurationFile.expected index c2164960179a..8c4b33e18309 100644 --- a/javascript/ql/test/query-tests/Security/CWE-313/PasswordInConfigurationFile.expected +++ b/javascript/ql/test/query-tests/Security/CWE-313/PasswordInConfigurationFile.expected @@ -1,2 +1,3 @@ -| mysql-config.json:4:16:4:23 | "secret" | Avoid plaintext passwords in configuration files. | -| tst4.json:2:10:2:38 | "script ... ecret'" | Avoid plaintext passwords in configuration files. | +| mysql-config.json:4:16:4:23 | "secret" | Hard-coded password 'secret' in configuration file. | +| tst4.json:2:10:2:38 | "script ... ecret'" | Hard-coded password ''secret'' in configuration file. | +| tst7.yml:2:9:2:6 | \| | Hard-coded password 'abc' in configuration file. | diff --git a/javascript/ql/test/query-tests/Security/CWE-313/tst7.yml b/javascript/ql/test/query-tests/Security/CWE-313/tst7.yml index 5813ec0dd377..336ef4165d52 100644 --- a/javascript/ql/test/query-tests/Security/CWE-313/tst7.yml +++ b/javascript/ql/test/query-tests/Security/CWE-313/tst7.yml @@ -1 +1,7 @@ password: $$SOME_VAR +config: | + [mail] + host = smtp.mydomain.com + port = 25 + username = sample_admin@mydomain.com + password = abc diff --git a/javascript/ql/test/query-tests/Security/CWE-313/tst8.yml b/javascript/ql/test/query-tests/Security/CWE-313/tst8.yml new file mode 100644 index 000000000000..188798fe3319 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-313/tst8.yml @@ -0,0 +1,6 @@ +config: | + [mail] + host = smtp.mydomain.com + port = 25 + username = {{username}} + password = {{pwd}}