Merge pull request github#12679 from ctbellanti/improved-certificate-validation

erik-krogh · web-flow · commit b38246507863 · 2023-03-30T16:24:33.000+02:00
JS: Improved coverage for disabled certificate validation
diff --git a/javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql b/javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql
@@ -19,6 +19,8 @@ DataFlow::InvokeNode tlsInvocation() {
   or
   result = DataFlow::moduleMember("https", "Agent").getAnInstantiation()
   or
+  result = DataFlow::moduleMember("https", "createServer").getACall()
+  or
   exists(DataFlow::NewNode new |
     new = DataFlow::moduleMember("tls", "TLSSocket").getAnInstantiation()
   |
diff --git a/javascript/ql/src/change-notes/2023-03-27-disabling-certificate-validation.md b/javascript/ql/src/change-notes/2023-03-27-disabling-certificate-validation.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `DisablingCertificateValidation.ql` query has been updated to check `createServer` from `https` for disabled certificate validation.
diff --git a/javascript/ql/test/query-tests/Security/CWE-295/DisablingCertificateValidation.expected b/javascript/ql/test/query-tests/Security/CWE-295/DisablingCertificateValidation.expected
@@ -9,3 +9,4 @@
 | 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. |
+| tst.js:80:5:80:29 | rejectU ... : false | Disabling certificate validation is strongly discouraged. |
diff --git a/javascript/ql/test/query-tests/Security/CWE-295/tst.js b/javascript/ql/test/query-tests/Security/CWE-295/tst.js
@@ -74,4 +74,8 @@ function getSomeunsafeOptions() {
         rejectUnauthorized: false // NOT OK
     }
 }
-new https.Agent(getSomeunsafeOptions());
+new https.Agent(getSomeunsafeOptions());
+
+https.createServer({
+    rejectUnauthorized: false // NOT OK
+});

Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,8 @@ DataFlow::InvokeNode tlsInvocation() {`
`19`	`19`	`or`
`20`	`20`	`result = DataFlow::moduleMember("https", "Agent").getAnInstantiation()`
`21`	`21`	`or`
	`22`	`+ result = DataFlow::moduleMember("https", "createServer").getACall()`
	`23`	`+ or`
`22`	`24`	`exists(DataFlow::NewNode new \|`
`23`	`25`	`new = DataFlow::moduleMember("tls", "TLSSocket").getAnInstantiation()`
`24`	`26`	`\|`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +---
 +category: minorAnalysis
 +---
 +* The `DisablingCertificateValidation.ql` query has been updated to check `createServer` from `https` for disabled certificate validation.
Original file line number	Diff line number	Diff line change
`@@ -74,4 +74,8 @@ function getSomeunsafeOptions() {`
`74`	`74`	`rejectUnauthorized: false // NOT OK`
`75`	`75`	`}`
`76`	`76`	`}`
`77`		`-new https.Agent(getSomeunsafeOptions());`
	`77`	`+new https.Agent(getSomeunsafeOptions());`
	`78`	`+`
	`79`	`+https.createServer({`
	`80`	`+ rejectUnauthorized: false // NOT OK`
	`81`	`+});`