Merge pull request #181 from sauyon/hardcoded-sensitive

Max Schaefer · GitHub Enterprise · commit 50cde34878c2 · 2019-11-13T09:21:45.000Z
HardcodedCredentials: Use SensitiveActions
diff --git a/ql/src/Security/CWE-798/HardcodedCredentials.go b/ql/src/Security/CWE-798/HardcodedCredentials.go
@@ -11,7 +11,7 @@ import (
 
 const (
 	user     = "dbuser"
-	password = "secretpassword"
+	password = "s3cretp4ssword"
 )
 
 func connect() *sql.DB {
diff --git a/ql/src/Security/CWE-798/HardcodedCredentials.ql b/ql/src/Security/CWE-798/HardcodedCredentials.ql
@@ -13,37 +13,31 @@
  */
 
 import go
+import semmle.go.security.SensitiveActions
 
 /**
  * Holds if `sink` is used in a context that suggests it may hold sensitive data of
  * the given `type`.
  */
-predicate isSensitive(DataFlow::Node sink, string type) {
+predicate isSensitive(DataFlow::Node sink, SensitiveExpr::Classification type) {
   exists(Write write, string name |
     write.getRhs() = sink and
     name = write.getLhs().getName() and
     // whitelist obvious test password variables
-    not name.regexpMatch("(?i)test.*")
+    not name.regexpMatch(HeuristicNames::notSensitive())
   |
-    name.regexpMatch("(?i)_*secret") and
-    type = "secret"
-    or
-    name.regexpMatch("(?i)_*(secret|access|private|rsa|aes)_*key") and
-    type = "key"
-    or
-    name.regexpMatch("(?i)_*(encrypted|old|new)?_*pass(wd|word|code|phrase)_*(chars|value)?") and
-    type = "password"
+    name.regexpMatch(HeuristicNames::maybeSensitive(type))
   )
 }
 
-from DataFlow::Node source, string message, DataFlow::Node sink, string type
+from DataFlow::Node source, string message, DataFlow::Node sink, SensitiveExpr::Classification type
 where
   exists(string val | val = source.getStringValue() and val != "" |
     isSensitive(sink, type) and
     DataFlow::localFlow(source, sink) and
     // whitelist obvious dummy/test values
-    not val.regexpMatch("(?i)test|password|secret|--- redacted ---") and
-    not sink.asExpr().(Ident).getName().regexpMatch("(?i)test.*")
+    not PasswordHeuristics::isDummyPassword(val) and
+    not sink.asExpr().(Ident).getName().regexpMatch(HeuristicNames::notSensitive())
   ) and
   message = "Hard-coded $@."
   or
@@ -52,6 +46,6 @@ where
       .regexpMatch("(?s)-+BEGIN\\b.*\\bPRIVATE KEY-+.+-+END\\b.*\\bPRIVATE KEY-+\n?") and
   (source.asExpr() instanceof StringLit or source.asExpr() instanceof AddExpr) and
   sink = source and
-  type = "" and
+  type = SensitiveExpr::certificate() and
   message = "Hard-coded private key."
-select sink, message, source, type
+select sink, message, source, type.toString()
diff --git a/ql/src/semmle/go/security/SensitiveActions.qll b/ql/src/semmle/go/security/SensitiveActions.qll
@@ -63,10 +63,10 @@ module HeuristicNames {
 
   /**
    * Gets a regular expression that identifies strings that may indicate the presence of data
-   * that is hashed or encrypted, and hence rendered non-sensitive.
+   * that is hashed, encrypted, or a test value, and hence non-sensitive.
    */
   string notSensitive() {
-    result = "(?is).*(redact|censor|obfuscate|hash|md5|sha|((?<!un)(en))?(crypt|code)).*"
+    result = "(?is).*(test|redact|censor|obfuscate|hash|md5|sha|((?<!un)(en))?(crypt|code)).*"
   }
 }
 
@@ -244,7 +244,8 @@ module PasswordHeuristics {
     or
     exists(string normalized | normalized = password.toLowerCase() |
       count(normalized.charAt(_)) = 1 or
-      normalized.regexpMatch(".*(pass|test|sample|example|secret|root|admin|user|change|auth).*")
+      normalized
+          .regexpMatch(".*(pass|test|sample|example|secret|root|admin|user|change|auth|redacted).*")
     )
   }
 }
diff --git a/ql/test/query-tests/Security/CWE-798/HardcodedCredentials.expected b/ql/test/query-tests/Security/CWE-798/HardcodedCredentials.expected
@@ -1,7 +1,7 @@
 | AlertSuppressionExample.go:11:14:11:40 | "horsebatterystaplecorrect" | Hard-coded $@. | AlertSuppressionExample.go:11:14:11:40 | "horsebatterystaplecorrect" | password |
-| HardcodedCredentials.go:10:13:10:28 | "secretpassword" | Hard-coded $@. | HardcodedCredentials.go:10:13:10:28 | "secretpassword" | password |
-| main.go:6:17:6:26 | "passw0rd" | Hard-coded $@. | main.go:6:17:6:26 | "passw0rd" | password |
-| main.go:13:1:27:30 | `-----BEGIN RSA PRIVATE KEY-----\nMIICXQIBAAKBgQC/tzdtXKXcX6F3v3hR6+uYyZpIeXhhLflJkY2eILLQfAnwKlT5\nxIHW5QZcHQV9sCyZ8qSdPGif7PwgMbButMbByiZhCSugUFb6vjVqoktmslYF4LKH\niDgvmlwuJW0TvynxBLzDCwrRP+gpRT8wuAortWAx/03POTw7Mzi2cIPNsQIDAQAB\nAoGAMHCrqY9CPTdQhgAz94cDpTwzJmLCvtMt7J/BR5X9eF4O6MbZZ652HAUMIVQX\n4hUUf+VmIHB2AwqO/ddwO9ijaz04BslOSy/iYevHGlH65q4587NSlFWjvILMIQCM\nGBjfzJIxlLHVhjc2cFnyAE5YWjF/OMnJN0OhP9pxmCP/iM0CQQDxmQndQLdnV7+6\n8SvBHE8bg1LE8/BzTt68U3aWwiBjrHMFgzr//7Za4VF7h4ilFgmbh0F3sYz+C8iO\n0JrBRPeLAkEAyyTwnv/pgqTS/wuxIHUxRBpbdk3YvILAthNrGQg5uzA7eSeFu7Mv\nGtEkXsaqCDbdehgarFfNN8PB6OMRIbsXMwJBAOjhH8UJ0L/osYO9XPO0GfznRS1c\nBnbfm4vk1/bSAO6TF/xEVubU0i4f6q8sIecfqvskEVMS7lkjeptPMR0DIakCQE+7\nuQH/Wizf+r0GXshplyOu4LVHisk63N7aMlAJ7XbuUHmWLKRmiReSfR8CBNzig/2X\nFmkMsUyw9hwte5zsrQcCQQCrOkZvzUj9j1HKG+32EJ2E4kisJZmAgF9GI+z6oxpi\nExped5tp8EWytCjRwKhOcc0068SgaqhKvyyUWpbx32VQ\n-----END RSA PRIVATE KEY-----` | Hard-coded private key. | main.go:13:1:27:30 | `-----BEGIN RSA PRIVATE KEY-----\nMIICXQIBAAKBgQC/tzdtXKXcX6F3v3hR6+uYyZpIeXhhLflJkY2eILLQfAnwKlT5\nxIHW5QZcHQV9sCyZ8qSdPGif7PwgMbButMbByiZhCSugUFb6vjVqoktmslYF4LKH\niDgvmlwuJW0TvynxBLzDCwrRP+gpRT8wuAortWAx/03POTw7Mzi2cIPNsQIDAQAB\nAoGAMHCrqY9CPTdQhgAz94cDpTwzJmLCvtMt7J/BR5X9eF4O6MbZZ652HAUMIVQX\n4hUUf+VmIHB2AwqO/ddwO9ijaz04BslOSy/iYevHGlH65q4587NSlFWjvILMIQCM\nGBjfzJIxlLHVhjc2cFnyAE5YWjF/OMnJN0OhP9pxmCP/iM0CQQDxmQndQLdnV7+6\n8SvBHE8bg1LE8/BzTt68U3aWwiBjrHMFgzr//7Za4VF7h4ilFgmbh0F3sYz+C8iO\n0JrBRPeLAkEAyyTwnv/pgqTS/wuxIHUxRBpbdk3YvILAthNrGQg5uzA7eSeFu7Mv\nGtEkXsaqCDbdehgarFfNN8PB6OMRIbsXMwJBAOjhH8UJ0L/osYO9XPO0GfznRS1c\nBnbfm4vk1/bSAO6TF/xEVubU0i4f6q8sIecfqvskEVMS7lkjeptPMR0DIakCQE+7\nuQH/Wizf+r0GXshplyOu4LVHisk63N7aMlAJ7XbuUHmWLKRmiReSfR8CBNzig/2X\nFmkMsUyw9hwte5zsrQcCQQCrOkZvzUj9j1HKG+32EJ2E4kisJZmAgF9GI+z6oxpi\nExped5tp8EWytCjRwKhOcc0068SgaqhKvyyUWpbx32VQ\n-----END RSA PRIVATE KEY-----` |  |
-| main.go:45:14:45:19 | "pass" | Hard-coded $@. | main.go:45:14:45:19 | "pass" | password |
-| main.go:49:13:49:15 | tmp | Hard-coded $@. | main.go:45:14:45:19 | "pass" | password |
-| main.go:51:15:51:21 | "pass2" | Hard-coded $@. | main.go:51:15:51:21 | "pass2" | password |
+| HardcodedCredentials.go:10:13:10:28 | "s3cretp4ssword" | Hard-coded $@. | HardcodedCredentials.go:10:13:10:28 | "s3cretp4ssword" | password |
+| main.go:6:14:6:23 | "p4ssw0rd" | Hard-coded $@. | main.go:6:14:6:23 | "p4ssw0rd" | password |
+| main.go:12:1:26:30 | `-----BEGIN RSA PRIVATE KEY-----\nMIICXQIBAAKBgQC/tzdtXKXcX6F3v3hR6+uYyZpIeXhhLflJkY2eILLQfAnwKlT5\nxIHW5QZcHQV9sCyZ8qSdPGif7PwgMbButMbByiZhCSugUFb6vjVqoktmslYF4LKH\niDgvmlwuJW0TvynxBLzDCwrRP+gpRT8wuAortWAx/03POTw7Mzi2cIPNsQIDAQAB\nAoGAMHCrqY9CPTdQhgAz94cDpTwzJmLCvtMt7J/BR5X9eF4O6MbZZ652HAUMIVQX\n4hUUf+VmIHB2AwqO/ddwO9ijaz04BslOSy/iYevHGlH65q4587NSlFWjvILMIQCM\nGBjfzJIxlLHVhjc2cFnyAE5YWjF/OMnJN0OhP9pxmCP/iM0CQQDxmQndQLdnV7+6\n8SvBHE8bg1LE8/BzTt68U3aWwiBjrHMFgzr//7Za4VF7h4ilFgmbh0F3sYz+C8iO\n0JrBRPeLAkEAyyTwnv/pgqTS/wuxIHUxRBpbdk3YvILAthNrGQg5uzA7eSeFu7Mv\nGtEkXsaqCDbdehgarFfNN8PB6OMRIbsXMwJBAOjhH8UJ0L/osYO9XPO0GfznRS1c\nBnbfm4vk1/bSAO6TF/xEVubU0i4f6q8sIecfqvskEVMS7lkjeptPMR0DIakCQE+7\nuQH/Wizf+r0GXshplyOu4LVHisk63N7aMlAJ7XbuUHmWLKRmiReSfR8CBNzig/2X\nFmkMsUyw9hwte5zsrQcCQQCrOkZvzUj9j1HKG+32EJ2E4kisJZmAgF9GI+z6oxpi\nExped5tp8EWytCjRwKhOcc0068SgaqhKvyyUWpbx32VQ\n-----END RSA PRIVATE KEY-----` | Hard-coded private key. | main.go:12:1:26:30 | `-----BEGIN RSA PRIVATE KEY-----\nMIICXQIBAAKBgQC/tzdtXKXcX6F3v3hR6+uYyZpIeXhhLflJkY2eILLQfAnwKlT5\nxIHW5QZcHQV9sCyZ8qSdPGif7PwgMbButMbByiZhCSugUFb6vjVqoktmslYF4LKH\niDgvmlwuJW0TvynxBLzDCwrRP+gpRT8wuAortWAx/03POTw7Mzi2cIPNsQIDAQAB\nAoGAMHCrqY9CPTdQhgAz94cDpTwzJmLCvtMt7J/BR5X9eF4O6MbZZ652HAUMIVQX\n4hUUf+VmIHB2AwqO/ddwO9ijaz04BslOSy/iYevHGlH65q4587NSlFWjvILMIQCM\nGBjfzJIxlLHVhjc2cFnyAE5YWjF/OMnJN0OhP9pxmCP/iM0CQQDxmQndQLdnV7+6\n8SvBHE8bg1LE8/BzTt68U3aWwiBjrHMFgzr//7Za4VF7h4ilFgmbh0F3sYz+C8iO\n0JrBRPeLAkEAyyTwnv/pgqTS/wuxIHUxRBpbdk3YvILAthNrGQg5uzA7eSeFu7Mv\nGtEkXsaqCDbdehgarFfNN8PB6OMRIbsXMwJBAOjhH8UJ0L/osYO9XPO0GfznRS1c\nBnbfm4vk1/bSAO6TF/xEVubU0i4f6q8sIecfqvskEVMS7lkjeptPMR0DIakCQE+7\nuQH/Wizf+r0GXshplyOu4LVHisk63N7aMlAJ7XbuUHmWLKRmiReSfR8CBNzig/2X\nFmkMsUyw9hwte5zsrQcCQQCrOkZvzUj9j1HKG+32EJ2E4kisJZmAgF9GI+z6oxpi\nExped5tp8EWytCjRwKhOcc0068SgaqhKvyyUWpbx32VQ\n-----END RSA PRIVATE KEY-----` | certificate |
+| main.go:44:14:44:19 | "p4ss" | Hard-coded $@. | main.go:44:14:44:19 | "p4ss" | password |
+| main.go:48:13:48:15 | tmp | Hard-coded $@. | main.go:44:14:44:19 | "p4ss" | password |
+| main.go:50:15:50:21 | "p4ss2" | Hard-coded $@. | main.go:50:15:50:21 | "p4ss2" | password |
diff --git a/ql/test/query-tests/Security/CWE-798/HardcodedCredentials.go b/ql/test/query-tests/Security/CWE-798/HardcodedCredentials.go
@@ -7,7 +7,7 @@ import (
 
 const (
 	user     = "dbuser"
-	password = "secretpassword"
+	password = "s3cretp4ssword"
 )
 
 func connect() *sql.DB {
diff --git a/ql/test/query-tests/Security/CWE-798/main.go b/ql/test/query-tests/Security/CWE-798/main.go
@@ -3,9 +3,8 @@ package main
 import "fmt"
 
 const (
-	passwd       = "passw0rd" // NOT OK
-	notAPassword = "hello"    // OK
-	_password    = ""         // OK
+	passwd    = "p4ssw0rd" // NOT OK
+	_password = ""         // OK
 )
 
 // generated using http://travistidwell.com/jsencrypt/demo
@@ -42,15 +41,15 @@ type info struct {
 }
 
 func main() {
-	password := "pass" // NOT OK
+	password := "p4ss" // NOT OK
 	tmp := password
 	i := info{
 		username: "me",
 		password: tmp, // NOT OK
 	}
-	i.password = "pass2" // NOT OK
+	i.password = "p4ss2" // NOT OK
 	fmt.Println(password, i)
-	testPassword := "pass"          // OK
+	testPassword := "p4ss"          // OK
 	i.password = "test"             // OK
 	i.password = testPassword       // OK
 	secretKey = "secret"            // OK

Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ import (`
`11`	`11`
`12`	`12`	`const (`
`13`	`13`	`user = "dbuser"`
`14`		`- password = "secretpassword"`
	`14`	`+ password = "s3cretp4ssword"`
`15`	`15`	`)`
`16`	`16`
`17`	`17`	`func connect() *sql.DB {`
Original file line number	Diff line number	Diff line change
`@@ -63,10 +63,10 @@ module HeuristicNames {`
`63`	`63`
`64`	`64`	`/**`
`65`	`65`	`* Gets a regular expression that identifies strings that may indicate the presence of data`
`66`		`- * that is hashed or encrypted, and hence rendered non-sensitive.`
	`66`	`+ * that is hashed, encrypted, or a test value, and hence non-sensitive.`
`67`	`67`	`*/`
`68`	`68`	`string notSensitive() {`
`69`		`- result = "(?is).(redact\|censor\|obfuscate\|hash\|md5\|sha\|((?<!un)(en))?(crypt\|code))."`
	`69`	`+ result = "(?is).(test\|redact\|censor\|obfuscate\|hash\|md5\|sha\|((?<!un)(en))?(crypt\|code))."`
`70`	`70`	`}`
`71`	`71`	`}`
`72`	`72`
`@@ -244,7 +244,8 @@ module PasswordHeuristics {`
`244`	`244`	`or`
`245`	`245`	`exists(string normalized \| normalized = password.toLowerCase() \|`
`246`	`246`	`count(normalized.charAt(_)) = 1 or`
`247`		`- normalized.regexpMatch(".(pass\|test\|sample\|example\|secret\|root\|admin\|user\|change\|auth).")`
	`247`	`+ normalized`
	`248`	`+ .regexpMatch(".(pass\|test\|sample\|example\|secret\|root\|admin\|user\|change\|auth\|redacted).")`
`248`	`249`	`)`
`249`	`250`	`}`
`250`	`251`	`}`
Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@ import (`
`7`	`7`
`8`	`8`	`const (`
`9`	`9`	`user = "dbuser"`
`10`		`- password = "secretpassword"`
	`10`	`+ password = "s3cretp4ssword"`
`11`	`11`	`)`
`12`	`12`
`13`	`13`	`func connect() *sql.DB {`