Single HttpOnly query

Author: edvraa

csharp/ql/src/experimental/Security Features/CWE-1004/CookieHttpOnlyFalseAspNetCore.qhelp

@@ -20,17 +20,31 @@ them not accessible to JavaScript.
 <example>
 
 <p>
-In the example below to <code>Microsoft.AspNetCore.Http.CookieOptions.HttpOnly</code> is set to <code>true</code>.
+In the example below <code>Microsoft.AspNetCore.Http.CookieOptions.HttpOnly</code> is set to <code>true</code>.
 </p>
 
 <sample src="httponlyflagcore.cs" />
 
+<p>
+In the following example <code>CookiePolicyOptions</code> are set programmatically to configure defaults.
+</p>
+
+<sample src="cookiepolicyoptions.cs" />
+
+<p>
+In the example below <code>System.Web.HttpCookie.HttpOnly</code> is set to <code>true</code>.
+</p>
+
+<sample src="httponlyflag.cs" />
+
 </example>
 
 <references>
 
-<li>MSDN: <a href="https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.cookieoptions.httponly">CookieOptions.HttpOnly Property</a></li>
+<li><a href="https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.cookieoptions.httponly">CookieOptions.HttpOnly Property</a></li>
 <li><a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie">Set-Cookie</a> Header</li>
+<li><a href="https://msdn.microsoft.com/en-us/library/system.web.httpcookie.httponly(v=vs.110).aspx">HttpCookie.HttpOnly Property</a></li>
+<li><a href="https://msdn.microsoft.com/library/ms228262%28v=vs.100%29.aspx">httpCookies Element</a></li>
 
 </references>
 </qhelp>

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

@@ -6,46 +6,114 @@
  *              not accessible by JavaScript.
  * @kind problem
  * @problem.severity warning
- * @precision medium
- * @id cs/web/httponly-possibly-not-set-aspnetcore
+ * @precision high
+ * @id cs/web/httponly-not-set
  * @tags security
  *       external/cwe/cwe-1004
  */
 
 import csharp
+import semmle.code.asp.WebConfig
+import semmle.code.csharp.frameworks.system.Web
 import semmle.code.csharp.frameworks.microsoft.AspNetCore
 import semmle.code.csharp.dataflow.flowsources.AuthCookie
 
-from Call c, MicrosoftAspNetCoreHttpResponseCookies iResponse, MethodCall mc
+from Expr httpOnlySink
 where
-  // 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 exists(
-    OnAppendCookieHttpOnlyTrackingConfig config, DataFlow::Node source, DataFlow::Node sink
-  |
-    config.hasFlow(source, sink)
-  ) 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(
-        CookieOptionsTrackingConfiguration cookieTracking, DataFlow::Node creation,
-        DataFlow::Node append
-      |
-        cookieTracking.hasFlow(creation, append) and
-        creation.asExpr() = oc
+  exists(Assignment a, Expr val |
+    httpOnlySink = a.getRValue() and
+    val.getValue() = "false" 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 exists(
+              OnAppendCookieHttpOnlyTrackingConfig config, DataFlow::Node source,
+              DataFlow::Node sink
+            |
+              config.hasFlow(source, sink)
+            ) and
+            // Passed as third argument to `IResponseCookies.Append`
+            exists(
+              CookieOptionsTrackingConfiguration cookieTracking, DataFlow::Node creation,
+              DataFlow::Node append
+            |
+              cookieTracking.hasFlow(creation, append) and
+              creation.asExpr() = oc and
+              append.asExpr() = mc.getArgument(2)
+            )
+          )
+        )
+      )
+      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())
+      )
+    )
+  )
+  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 exists(
+          OnAppendCookieHttpOnlyTrackingConfig config, DataFlow::Node source, DataFlow::Node sink
+        |
+          config.hasFlow(source, sink)
+        ) 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(
+              CookieOptionsTrackingConfiguration cookieTracking, DataFlow::Node creation,
+              DataFlow::Node append
+            |
+              cookieTracking.hasFlow(creation, append) 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()
+        )
       )
     )
-    or
-    // IResponseCookies.Append(String, String) was called, `HttpOnly` is set to `false` by default
-    mc = c and
-    mc.getNumberOfArguments() < 3 and
-    isCookieWithSensitiveName(mc.getArgument(0))
   )
-select c, "Cookie attribute 'HttpOnly' is not set to true."
+select httpOnlySink, "Cookie attribute 'HttpOnly' is not set to true."