Merge pull request #8438 from erik-krogh/apiDisable

JS: add some API-nodes to js/disabling-certificate-validation

Author: erik-krogh

javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql

@@ -13,26 +13,25 @@
 
 import javascript
 
-/**
- * Gets an options object for a TLS connection.
- */
-DataFlow::ObjectLiteralNode tlsOptions() {
-  exists(DataFlow::InvokeNode invk | result.flowsTo(invk.getAnArgument()) |
-    invk instanceof ClientRequest
-    or
-    invk = DataFlow::moduleMember("https", "Agent").getAnInstantiation()
-    or
-    exists(DataFlow::NewNode new |
-      new = DataFlow::moduleMember("tls", "TLSSocket").getAnInstantiation()
-    |
-      invk = new or
-      invk = new.getAMethodCall("renegotiate")
-    )
-    or
-    invk = DataFlow::moduleMember("tls", ["connect", "createServer"]).getACall()
+/** Gets options argument for a potential TLS connection */
+DataFlow::InvokeNode tlsInvocation() {
+  result instanceof ClientRequest
+  or
+  result = DataFlow::moduleMember("https", "Agent").getAnInstantiation()
+  or
+  exists(DataFlow::NewNode new |
+    new = DataFlow::moduleMember("tls", "TLSSocket").getAnInstantiation()
+  |
+    result = new or
+    result = new.getAMethodCall("renegotiate")
   )
+  or
+  result = DataFlow::moduleMember("tls", ["connect", "createServer"]).getACall()
 }
 
+/** Gets an options object for a TLS connection. */
+DataFlow::ObjectLiteralNode tlsOptions() { result.flowsTo(tlsInvocation().getAnArgument()) }
+
 from DataFlow::PropWrite disable
 where
   exists(DataFlow::SourceNode env |
@@ -41,6 +40,13 @@ where
     disable.getRhs().mayHaveStringValue("0")
   )
   or
-  disable = tlsOptions().getAPropertyWrite("rejectUnauthorized") and
+  (
+    disable = tlsOptions().getAPropertyWrite("rejectUnauthorized")
+    or
+    // the same thing, but with API-nodes if they happen to be available
+    exists(API::Node tlsInvk | tlsInvk.getAnInvocation() = tlsInvocation() |
+      disable.getRhs() = tlsInvk.getAParameter().getMember("rejectUnauthorized").getARhs()
+    )
+  ) and
   disable.getRhs().(AnalyzedNode).getTheBooleanValue() = false
 select disable, "Disabling certificate validation is strongly discouraged."

javascript/ql/test/query-tests/Security/CWE-295/DisablingCertificateValidation.expected

@@ -8,3 +8,4 @@
 | tst.js:39:2:39:29 | rejectU ... ndirect | Disabling certificate validation is strongly discouraged. |
 | tst.js:45:2:45:28 | rejectU ... !!false | Disabling certificate validation is strongly discouraged. |
 | tst.js:48:2:48:26 | rejectU ... : !true | Disabling certificate validation is strongly discouraged. |
+| tst.js:74:9:74:33 | rejectU ... : false | Disabling certificate validation is strongly discouraged. |

javascript/ql/test/query-tests/Security/CWE-295/tst.js

@@ -68,3 +68,10 @@ new https.Agent({
 new https.Agent({
 	rejectUnauthorized: typeof getOptions().rejectUnauthorized === 'boolean' ? getOptions().rejectUnauthorized : undefined // OK
 });
+
+function getSomeunsafeOptions() {
+    return {
+        rejectUnauthorized: false // NOT OK
+    }
+}
+new https.Agent(getSomeunsafeOptions());