Get rid of NetHttpCookieTrackingConfiguration

Author: edvraa

ql/src/experimental/CWE-1004/AuthCookie.qll

@@ -25,7 +25,7 @@ private class GorillaSessionOptionsField extends Field {
  *
  * This should cover most typical patterns...
  */
-DataFlow::Node getValueForFieldWrite(StructLit sl, string field) {
+private DataFlow::Node getValueForFieldWrite(StructLit sl, string field) {
   exists(Write w, DataFlow::Node base, Field f |
     f.getName() = field and
     w.writesField(base, f, result) and
@@ -64,32 +64,10 @@ private class SetCookieSink extends DataFlow::Node {
   }
 }
 
-/**
- * Tracks `net/http.Cookie` creation to `net/http.SetCookie`.
- */
-class NetHttpCookieTrackingConfiguration extends TaintTracking::Configuration {
-  NetHttpCookieTrackingConfiguration() { this = "NetHttpCookieTrackingConfiguration" }
-
-  override predicate isSource(DataFlow::Node source) {
-    exists(StructLit sl |
-      source.asExpr() = sl and
-      sl.getType() instanceof NetHttpCookieType
-    )
-  }
-
-  override predicate isSink(DataFlow::Node sink) {
-    sink instanceof SetCookieSink and
-    exists(NameToNetHttpCookieTrackingConfiguration cfg, DataFlow::Node nameArg |
-      cfg.hasFlowTo(nameArg) and
-      sink.asExpr() = nameArg.asExpr()
-    )
-  }
-}
-
 /**
  * Tracks sensitive name to `net/http.SetCookie`.
  */
-private class NameToNetHttpCookieTrackingConfiguration extends TaintTracking2::Configuration {
+class NameToNetHttpCookieTrackingConfiguration extends TaintTracking::Configuration {
   NameToNetHttpCookieTrackingConfiguration() { this = "NameToNetHttpCookieTrackingConfiguration" }
 
   override predicate isSource(DataFlow::Node source) { isAuthVariable(source.asExpr()) }
@@ -106,12 +84,14 @@ private class NameToNetHttpCookieTrackingConfiguration extends TaintTracking2::C
 }
 
 /**
- * Tracks `HttpOnly` set to `false` to `net/http.SetCookie`.
+ * Tracks `bool` assigned to `HttpOnly` that flows into `net/http.SetCookie`.
  */
 class BoolToNetHttpCookieTrackingConfiguration extends TaintTracking::Configuration {
   BoolToNetHttpCookieTrackingConfiguration() { this = "BoolToNetHttpCookieTrackingConfiguration" }
 
-  override predicate isSource(DataFlow::Node source) { source.asExpr().getBoolValue() = false }
+  override predicate isSource(DataFlow::Node source) {
+    source.asExpr().getType().getUnderlyingType() instanceof BoolType
+  }
 
   override predicate isSink(DataFlow::Node sink) { sink instanceof SetCookieSink }
 

ql/src/experimental/CWE-1004/CookieWithoutHttpOnly.ql

@@ -18,15 +18,20 @@ import DataFlow::PathGraph
 
 /** Holds if `HttpOnly` of `net/http.SetCookie` is set to `false` or not set (default value is used). */
 predicate isNetHttpCookieFlow(DataFlow::PathNode source, DataFlow::PathNode sink) {
-  exists(DataFlow::PathNode cookieCreate, DataFlow::PathNode setCookieSink |
-    exists(NetHttpCookieTrackingConfiguration cfg | cfg.hasFlowPath(cookieCreate, setCookieSink)) and
+  exists(DataFlow::PathNode sensitiveName, DataFlow::PathNode setCookieSink |
+    exists(NameToNetHttpCookieTrackingConfiguration cfg |
+      cfg.hasFlowPath(sensitiveName, setCookieSink)
+    ) and
     (
-      not exists(getValueForFieldWrite(cookieCreate.getNode().asExpr(), "HttpOnly")) and
-      source = cookieCreate and
+      not exists(BoolToNetHttpCookieTrackingConfiguration cfg |
+        cfg.hasFlowTo(setCookieSink.getNode())
+      ) and
+      source = sensitiveName and
       sink = setCookieSink
       or
       exists(BoolToNetHttpCookieTrackingConfiguration cfg |
         cfg.hasFlow(source.getNode(), setCookieSink.getNode()) and
+        source.getNode().getBoolValue() = false and
         sink = setCookieSink
       )
     )