JS: sharpen js/clear-text-logging (ODASA-7485)

Author: Esben Sparre Andreasen

change-notes/1.19/analysis-javascript.md

@@ -36,6 +36,7 @@
 | **Query**                      | **Expected impact**        | **Change**                                   |
 |--------------------------------|----------------------------|----------------------------------------------|
 | Ambiguous HTML id attribute | Lower severity | The severity of this rule has been revised to "warning". |
+| Clear-text logging of sensitive information | Fewer results | This rule now tracks flow more precisely. |
 | Client side cross-site scripting | More results | This rule now also flags HTML injection in the body of an email. |
 | Client-side URL redirect | Fewer false-positive results | This rule now recognizes safe redirects in more cases. |
 | Conflicting HTML element attributes | Lower severity | The severity of this rule has been revised to "warning". |

javascript/ql/src/semmle/javascript/security/dataflow/CleartextLogging.qll

@@ -52,7 +52,7 @@ module CleartextLogging {
     }
 
     override predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg) {
-      any (TaintTracking::StringConcatenationTaintStep s).step(src, trg)
+      StringConcatenation::taintStep(src, trg)
       or
       exists (string name | name = "toString" or name = "valueOf" |
         src.(DataFlow::SourceNode).getAMethodCall(name) = trg

javascript/ql/test/query-tests/Security/CWE-312/CleartextLogging.expected

@@ -22,10 +22,7 @@ nodes
 | passwords.js:28:16:28:17 | {} |
 | passwords.js:29:17:29:20 | obj3 |
 | passwords.js:30:14:30:21 | password |
-| passwords.js:77:9:77:55 | temp |
-| passwords.js:77:16:77:55 | { encry ... sword } |
 | passwords.js:77:37:77:53 | req.body.password |
-| passwords.js:78:17:78:20 | temp |
 | passwords.js:78:17:78:38 | temp.en ... assword |
 | passwords.js:80:9:80:25 | secret |
 | passwords.js:80:18:80:25 | password |
@@ -49,6 +46,13 @@ nodes
 | passwords.js:123:17:123:48 | name +  ... lueOf() |
 | passwords.js:123:31:123:38 | password |
 | passwords.js:123:31:123:48 | password.valueOf() |
+| passwords.js:127:9:132:5 | config |
+| passwords.js:127:18:132:5 | {\\n      ... )\\n    } |
+| passwords.js:130:12:130:19 | password |
+| passwords.js:131:12:131:24 | getPassword() |
+| passwords.js:135:17:135:22 | config |
+| passwords.js:136:17:136:24 | config.x |
+| passwords.js:137:17:137:24 | config.y |
 | passwords_in_browser1.js:2:13:2:20 | password |
 | passwords_in_browser2.js:2:13:2:20 | password |
 | passwords_in_server_1.js:6:13:6:20 | password |
@@ -71,11 +75,7 @@ edges
 | passwords.js:28:9:28:17 | obj3 | passwords.js:29:17:29:20 | obj3 |
 | passwords.js:28:16:28:17 | {} | passwords.js:28:9:28:17 | obj3 |
 | passwords.js:30:14:30:21 | password | passwords.js:28:16:28:17 | {} |
-| passwords.js:77:9:77:55 | temp | passwords.js:78:17:78:20 | temp |
-| passwords.js:77:16:77:55 | { encry ... sword } | passwords.js:77:9:77:55 | temp |
-| passwords.js:77:37:77:53 | req.body.password | passwords.js:77:16:77:55 | { encry ... sword } |
 | passwords.js:77:37:77:53 | req.body.password | passwords.js:78:17:78:38 | temp.en ... assword |
-| passwords.js:78:17:78:20 | temp | passwords.js:78:17:78:38 | temp.en ... assword |
 | passwords.js:80:9:80:25 | secret | passwords.js:81:24:81:29 | secret |
 | passwords.js:80:18:80:25 | password | passwords.js:80:9:80:25 | secret |
 | passwords.js:81:24:81:29 | secret | passwords.js:81:17:81:31 | `pw: ${secret}` |
@@ -89,6 +89,12 @@ edges
 | passwords.js:122:31:122:49 | password.toString() | passwords.js:122:17:122:49 | name +  ... tring() |
 | passwords.js:123:31:123:38 | password | passwords.js:123:31:123:48 | password.valueOf() |
 | passwords.js:123:31:123:48 | password.valueOf() | passwords.js:123:17:123:48 | name +  ... lueOf() |
+| passwords.js:127:9:132:5 | config | passwords.js:135:17:135:22 | config |
+| passwords.js:127:18:132:5 | {\\n      ... )\\n    } | passwords.js:127:9:132:5 | config |
+| passwords.js:130:12:130:19 | password | passwords.js:127:18:132:5 | {\\n      ... )\\n    } |
+| passwords.js:130:12:130:19 | password | passwords.js:136:17:136:24 | config.x |
+| passwords.js:131:12:131:24 | getPassword() | passwords.js:127:18:132:5 | {\\n      ... )\\n    } |
+| passwords.js:131:12:131:24 | getPassword() | passwords.js:137:17:137:24 | config.y |
 | passwords_in_server_5.js:4:7:4:24 | req.query.password | passwords_in_server_5.js:7:12:7:12 | x |
 | passwords_in_server_5.js:7:12:7:12 | x | passwords_in_server_5.js:8:17:8:17 | x |
 #select
@@ -113,6 +119,11 @@ edges
 | passwords.js:119:21:119:46 | "Passwo ... assword | passwords.js:119:39:119:46 | password | passwords.js:119:21:119:46 | "Passwo ... assword | Sensitive data returned by $@ is logged here. | passwords.js:119:39:119:46 | password | an access to password |
 | passwords.js:122:17:122:49 | name +  ... tring() | passwords.js:122:31:122:38 | password | passwords.js:122:17:122:49 | name +  ... tring() | Sensitive data returned by $@ is logged here. | passwords.js:122:31:122:38 | password | an access to password |
 | passwords.js:123:17:123:48 | name +  ... lueOf() | passwords.js:123:31:123:38 | password | passwords.js:123:17:123:48 | name +  ... lueOf() | Sensitive data returned by $@ is logged here. | passwords.js:123:31:123:38 | password | an access to password |
+| passwords.js:135:17:135:22 | config | passwords.js:127:18:132:5 | {\\n      ... )\\n    } | passwords.js:135:17:135:22 | config | Sensitive data returned by $@ is logged here. | passwords.js:127:18:132:5 | {\\n      ... )\\n    } | an access to password |
+| passwords.js:135:17:135:22 | config | passwords.js:130:12:130:19 | password | passwords.js:135:17:135:22 | config | Sensitive data returned by $@ is logged here. | passwords.js:130:12:130:19 | password | an access to password |
+| passwords.js:135:17:135:22 | config | passwords.js:131:12:131:24 | getPassword() | passwords.js:135:17:135:22 | config | Sensitive data returned by $@ is logged here. | passwords.js:131:12:131:24 | getPassword() | a call to getPassword |
+| passwords.js:136:17:136:24 | config.x | passwords.js:130:12:130:19 | password | passwords.js:136:17:136:24 | config.x | Sensitive data returned by $@ is logged here. | passwords.js:130:12:130:19 | password | an access to password |
+| passwords.js:137:17:137:24 | config.y | passwords.js:131:12:131:24 | getPassword() | passwords.js:137:17:137:24 | config.y | Sensitive data returned by $@ is logged here. | passwords.js:131:12:131:24 | getPassword() | a call to getPassword |
 | passwords_in_server_1.js:6:13:6:20 | password | passwords_in_server_1.js:6:13:6:20 | password | passwords_in_server_1.js:6:13:6:20 | password | Sensitive data returned by $@ is logged here. | passwords_in_server_1.js:6:13:6:20 | password | an access to password |
 | passwords_in_server_2.js:3:13:3:20 | password | passwords_in_server_2.js:3:13:3:20 | password | passwords_in_server_2.js:3:13:3:20 | password | Sensitive data returned by $@ is logged here. | passwords_in_server_2.js:3:13:3:20 | password | an access to password |
 | passwords_in_server_3.js:2:13:2:20 | password | passwords_in_server_3.js:2:13:2:20 | password | passwords_in_server_3.js:2:13:2:20 | password | Sensitive data returned by $@ is logged here. | passwords_in_server_3.js:2:13:2:20 | password | an access to password |

javascript/ql/test/query-tests/Security/CWE-312/passwords.js

@@ -122,3 +122,18 @@
     console.log(name + ", " + password.toString()); // NOT OK
     console.log(name + ", " + password.valueOf()); // NOT OK
 });
+
+(function() {
+    var config = {
+        password: x,
+        hostname: "tarski",
+        x: password,
+        y: getPassword()
+    };
+    var cfg = x? config: config;
+    console.log(config.hostname); // OK
+    console.log(config); // NOT OK
+    console.log(config.x); // NOT OK
+    console.log(config.y); // NOT OK
+    console.log(config[x]); // OK (probably)
+});