Broaden scope

python/ql/src/experimental/Security/CWE-614/InsecureCookie.ql

@@ -12,23 +12,16 @@
 // determine precision above
 import python
 import semmle.python.dataflow.new.DataFlow
-import semmle.python.Concepts
 import experimental.semmle.python.Concepts
 
-from Expr cookieExpr
+from Cookie cookie, string alert
 where
-  exists(HeaderDeclaration headerWrite, StrConst headerName, StrConst headerValue |
-    headerName.getText() = "Set-Cookie" and
-    DataFlow::exprNode(headerName).(DataFlow::LocalSourceNode).flowsTo(headerWrite.getNameArg()) and
-    not headerValue.getText().regexpMatch(".*; *Secure;.*") and
-    DataFlow::exprNode(headerValue).(DataFlow::LocalSourceNode).flowsTo(headerWrite.getValueArg()) and
-    cookieExpr = headerWrite.asExpr()
-  )
+  cookie.isSecure() and
+  alert = "secure"
   or
-  exists(ExperimentalHTTP::CookieWrite cookieWrite, False f, None n |
-    [DataFlow::exprNode(f), DataFlow::exprNode(n)]
-        .(DataFlow::LocalSourceNode)
-        .flowsTo(cookieWrite.(DataFlow::CallCfgNode).getArgByName("secure")) and
-    cookieExpr = cookieWrite.asExpr()
-  )
-select cookieExpr, "Cookie is added to response without the 'secure' flag being set."
+  not cookie.isHttpOnly() and
+  alert = "httponly"
+  or
+  cookie.isSameSite() and
+  alert = "samesite"
+select cookie, "Cookie is added without the ", alert, " flag properly set."

python/ql/src/experimental/semmle/python/Concepts.qll

@@ -301,54 +301,51 @@ class HeaderDeclaration extends DataFlow::Node {
  * A data-flow node that sets a cookie in an HTTP response.
  *
  * Extend this class to refine existing API models. If you want to model new APIs,
- * extend `HTTP::CookieWrite::Range` instead.
+ * extend `Cookie::Range` instead.
  */
-class CookieWrite extends DataFlow::Node {
-  CookieWrite::Range range;
+class Cookie extends DataFlow::Node {
+  Cookie::Range range;
 
-  CookieWrite() { this = range }
+  Cookie() { this = range }
 
   /**
-   * Gets the argument, if any, specifying the raw cookie header.
+   * Holds if this cookie is secure.
    */
-  DataFlow::Node getHeaderArg() { result = range.getHeaderArg() }
+  predicate isSecure() { range.isSecure() }
 
   /**
-   * Gets the argument, if any, specifying the cookie name.
+   * Holds if this cookie is HttpOnly.
    */
-  DataFlow::Node getNameArg() { result = range.getNameArg() }
+  predicate isHttpOnly() { range.isHttpOnly() }
 
   /**
-   * Gets the argument, if any, specifying the cookie value.
+   * Holds if the cookie is SameSite
    */
-  DataFlow::Node getValueArg() { result = range.getValueArg() }
+  predicate isSameSite() { range.isSameSite() }
 }
 
 /** Provides a class for modeling new cookie writes on HTTP responses. */
-module CookieWrite {
+module Cookie {
   /**
    * A data-flow node that sets a cookie in an HTTP response.
    *
-   * Note: we don't require that this redirect must be sent to a client (a kind of
-   * "if a tree falls in a forest and nobody hears it" situation).
-   *
    * Extend this class to model new APIs. If you want to refine existing API models,
-   * extend `HttpResponse` instead.
+   * extend `Cookie` instead.
    */
   abstract class Range extends DataFlow::Node {
     /**
-     * Gets the argument, if any, specifying the raw cookie header.
+     * Holds if this cookie is secure.
      */
-    abstract DataFlow::Node getHeaderArg();
+    abstract predicate isSecure();
 
     /**
-     * Gets the argument, if any, specifying the cookie name.
+     * Holds if this cookie is HttpOnly.
      */
-    abstract DataFlow::Node getNameArg();
+    abstract predicate isHttpOnly();
 
     /**
-     * Gets the argument, if any, specifying the cookie value.
+     * Holds if the cookie is SameSite.
      */
-    abstract DataFlow::Node getValueArg();
+    abstract predicate isSameSite();
   }
 }

python/ql/src/experimental/semmle/python/CookieHeader.qll

@@ -0,0 +1,26 @@
+/**
+ * Temporary: provides a class to extend current cookies to header declarations
+ */
+
+import python
+import semmle.python.dataflow.new.DataFlow
+import semmle.python.dataflow.new.TaintTracking
+import experimental.semmle.python.Concepts
+
+class CookieHeader extends HeaderDeclaration, Cookie::Range {
+  CookieHeader() {
+    this instanceof HeaderDeclaration and this.getNameArg().asExpr().(Str_).getS() = "Set-Cookie"
+  }
+
+  override predicate isSecure() {
+    this.getValueArg().asExpr().(Str_).getS().regexpMatch(".*; *Secure;.*")
+  }
+
+  override predicate isHttpOnly() {
+    this.getValueArg().asExpr().(Str_).getS().regexpMatch(".*; *HttpOnly;.*")
+  }
+
+  override predicate isSameSite() {
+    this.getValueArg().asExpr().(Str_).getS().regexpMatch(".*; *SameSite=(Strict|Lax);.*")
+  }
+}

python/ql/src/experimental/semmle/python/frameworks/Django.qll

@@ -87,15 +87,24 @@ private module PrivateDjango {
             override DataFlow::Node getValueArg() { result = headerInput }
           }
 
-          class DjangoSetCookieCall extends DataFlow::CallCfgNode,
-            ExperimentalHTTP::CookieWrite::Range {
+          class DjangoSetCookieCall extends DataFlow::CallCfgNode, Cookie::Range {
             DjangoSetCookieCall() { this = baseClassRef().getMember("set_cookie").getACall() }
 
-            override DataFlow::Node getHeaderArg() { none() }
+            override predicate isSecure() {
+              DataFlow::exprNode(any(True t))
+                  .(DataFlow::LocalSourceNode)
+                  .flowsTo(this.getArgByName("secure"))
+            }
 
-            override DataFlow::Node getNameArg() { result = this.getArg(0) }
+            override predicate isHttpOnly() {
+              DataFlow::exprNode(any(True t))
+                  .(DataFlow::LocalSourceNode)
+                  .flowsTo(this.getArgByName("httponly"))
+            }
 
-            override DataFlow::Node getValueArg() { result = this.getArg(1) }
+            override predicate isSameSite() {
+              this.getArgByName("samesite").asExpr().(Str_).getS() in ["Strict", "Lax"]
+            }
           }
         }
       }

python/ql/src/experimental/semmle/python/frameworks/Flask.qll

@@ -82,8 +82,8 @@ module ExperimentalFlask {
     override DataFlow::Node getValueArg() { result.asExpr() = item.getValue() }
   }
 
-  class DjangoSetCookieCall extends DataFlow::CallCfgNode, ExperimentalHTTP::CookieWrite::Range {
-    DjangoSetCookieCall() {
+  class FlaskSetCookieCall extends DataFlow::CallCfgNode, Cookie::Range {
+    FlaskSetCookieCall() {
       this =
         [Flask::Response::classRef(), flaskMakeResponse()]
             .getReturn()
@@ -91,10 +91,20 @@ module ExperimentalFlask {
             .getACall()
     }
 
-    override DataFlow::Node getHeaderArg() { none() }
+    override predicate isSecure() {
+      DataFlow::exprNode(any(True t))
+          .(DataFlow::LocalSourceNode)
+          .flowsTo(this.getArgByName("secure"))
+    }
 
-    override DataFlow::Node getNameArg() { result = this.getArg(0) }
+    override predicate isHttpOnly() {
+      DataFlow::exprNode(any(True t))
+          .(DataFlow::LocalSourceNode)
+          .flowsTo(this.getArgByName("httponly"))
+    }
 
-    override DataFlow::Node getValueArg() { result = this.getArg(1) }
+    override predicate isSameSite() {
+      this.getArgByName("samesite").asExpr().(Str_).getS() in ["Strict", "Lax"]
+    }
   }
 }