Refactor httponly cookie query

csharp/ql/src/experimental/Security Features/CWE-1004/CookieWithoutHttpOnly.ql

@@ -17,91 +17,103 @@ import csharp
 import semmle.code.asp.WebConfig
 import semmle.code.csharp.frameworks.system.Web
 import semmle.code.csharp.frameworks.microsoft.AspNetCore
-deprecated import experimental.dataflow.flowsources.AuthCookie
+import experimental.dataflow.flowsources.AuthCookie
 
-deprecated query predicate problems(Expr httpOnlySink, string message) {
+predicate cookieAppendHttpOnlyByDefault() {
+  // default is set to `Always`
+  getAValueForCookiePolicyProp("HttpOnly").getValue() = "1"
+  or
+  // there is an `OnAppendCookie` callback that sets `HttpOnly` to true
+  not OnAppendCookieHttpOnlyTracking::flowTo(_)
+}
+
+predicate httpOnlyFalse(ObjectCreation oc) {
+  exists(Assignment a |
+    getAValueForProp(oc, a, "HttpOnly") = a.getRValue() and
+    a.getRValue().getValue() = "false"
+  )
+}
+
+predicate httpOnlyFalseOrNotSet(ObjectCreation oc) {
+  httpOnlyFalse(oc)
+  or
+  not isPropertySet(oc, "HttpOnly")
+}
+
+predicate nonHttpOnlyCookieOptionsCreation(ObjectCreation oc, MethodCall append) {
+  // `HttpOnly` property in `CookieOptions` passed to IResponseCookies.Append(...) wasn't set
+  oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and
+  httpOnlyFalseOrNotSet(oc) and
+  exists(DataFlow::Node creation, DataFlow::Node sink |
+    CookieOptionsTracking::flow(creation, sink) and
+    creation.asExpr() = oc and
+    sink.asExpr() = append.getArgument(2)
+  )
+}
+
+predicate nonHttpOnlySensitiveCookieCreation(ObjectCreation oc) {
+  oc.getType() instanceof SystemWebHttpCookie and
+  isCookieWithSensitiveName(oc.getArgument(0)) and
   (
-    exists(Assignment a, Expr val |
-      httpOnlySink = a.getRValue() and
-      val.getValue() = "false" and
+    httpOnlyFalse(oc)
+    or
+    // the property wasn't explicitly set, so a default value from config is used
+    not isPropertySet(oc, "HttpOnly") and
+    // the default in config is not set to `true`
+    not exists(XmlElement element |
+      element instanceof HttpCookiesElement and
+      element.(HttpCookiesElement).isHttpOnlyCookies()
+    )
+  )
+}
+
+predicate sensitiveCookieAppend(MethodCall mc) {
+  exists(MicrosoftAspNetCoreHttpResponseCookies iResponse |
+    iResponse.getAppendMethod() = mc.getTarget() and
+    isCookieWithSensitiveName(mc.getArgument(0))
+  )
+}
+
+predicate nonHttpOnlyCookieCall(Call c) {
+  (
+    not cookieAppendHttpOnlyByDefault() and
+    exists(MethodCall mc |
+      sensitiveCookieAppend(mc) and
       (
-        exists(ObjectCreation oc |
-          getAValueForProp(oc, a, "HttpOnly") = val and
-          (
-            oc.getType() instanceof SystemWebHttpCookie and
-            isCookieWithSensitiveName(oc.getArgument(0))
-            or
-            exists(MethodCall mc, MicrosoftAspNetCoreHttpResponseCookies iResponse |
-              oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and
-              iResponse.getAppendMethod() = mc.getTarget() and
-              isCookieWithSensitiveName(mc.getArgument(0)) and
-              // there is no callback `OnAppendCookie` that sets `HttpOnly` to true
-              not OnAppendCookieHttpOnlyTracking::flowTo(_) and
-              // Passed as third argument to `IResponseCookies.Append`
-              exists(DataFlow::Node creation, DataFlow::Node append |
-                CookieOptionsTracking::flow(creation, append) and
-                creation.asExpr() = oc and
-                append.asExpr() = mc.getArgument(2)
-              )
-            )
-          )
-        )
+        nonHttpOnlyCookieOptionsCreation(c, mc)
         or
-        exists(PropertyWrite pw |
-          (
-            pw.getProperty().getDeclaringType() instanceof MicrosoftAspNetCoreHttpCookieBuilder or
-            pw.getProperty().getDeclaringType() instanceof
-              MicrosoftAspNetCoreAuthenticationCookiesCookieAuthenticationOptions
-          ) and
-          pw.getProperty().getName() = "HttpOnly" and
-          a.getLValue() = pw and
-          DataFlow::localExprFlow(val, a.getRValue())
-        )
+        // IResponseCookies.Append(String, String) was called, `HttpOnly` is set to `false` by default
+        mc = c and
+        mc.getNumberOfArguments() < 3
       )
     )
     or
-    exists(Call c |
-      httpOnlySink = c and
-      (
-        exists(MicrosoftAspNetCoreHttpResponseCookies iResponse, MethodCall mc |
-          // default is not configured or is not set to `Always`
-          not getAValueForCookiePolicyProp("HttpOnly").getValue() = "1" and
-          // there is no callback `OnAppendCookie` that sets `HttpOnly` to true
-          not OnAppendCookieHttpOnlyTracking::flowTo(_) and
-          iResponse.getAppendMethod() = mc.getTarget() and
-          isCookieWithSensitiveName(mc.getArgument(0)) and
-          (
-            // `HttpOnly` property in `CookieOptions` passed to IResponseCookies.Append(...) wasn't set
-            exists(ObjectCreation oc |
-              oc = c and
-              oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and
-              not isPropertySet(oc, "HttpOnly") and
-              exists(DataFlow::Node creation |
-                CookieOptionsTracking::flow(creation, _) and
-                creation.asExpr() = oc
-              )
-            )
-            or
-            // IResponseCookies.Append(String, String) was called, `HttpOnly` is set to `false` by default
-            mc = c and
-            mc.getNumberOfArguments() < 3
-          )
-        )
-        or
-        exists(ObjectCreation oc |
-          oc = c and
-          oc.getType() instanceof SystemWebHttpCookie and
-          isCookieWithSensitiveName(oc.getArgument(0)) and
-          // the property wasn't explicitly set, so a default value from config is used
-          not isPropertySet(oc, "HttpOnly") and
-          // the default in config is not set to `true`
-          not exists(XmlElement element |
-            element instanceof HttpCookiesElement and
-            element.(HttpCookiesElement).isHttpOnlyCookies()
-          )
-        )
-      )
-    )
-  ) and
-  message = "Cookie attribute 'HttpOnly' is not set to true."
+    nonHttpOnlySensitiveCookieCreation(c)
+  )
 }
+
+predicate nonHttpOnlyPolicyAssignment(Assignment a, Expr val) {
+  val.getValue() = "false" and
+  exists(PropertyWrite pw |
+    (
+      pw.getProperty().getDeclaringType() instanceof MicrosoftAspNetCoreHttpCookieBuilder or
+      pw.getProperty().getDeclaringType() instanceof
+        MicrosoftAspNetCoreAuthenticationCookiesCookieAuthenticationOptions
+    ) and
+    pw.getProperty().getName() = "HttpOnly" and
+    a.getLValue() = pw and
+    DataFlow::localExprFlow(val, a.getRValue())
+  )
+}
+
+from Expr httpOnlySink
+where
+  (
+    nonHttpOnlyCookieCall(httpOnlySink)
+    or
+    exists(Assignment a |
+      httpOnlySink = a.getRValue() and
+      nonHttpOnlyPolicyAssignment(a, _)
+    )
+  )
+select httpOnlySink, "Cookie attribute 'HttpOnly' is not set to true."