diff --git a/python/ql/src/experimental/Security/CWE-614/InsecureCookie.py b/python/ql/src/experimental/Security/CWE-614/InsecureCookie.py deleted file mode 100644 index 4087830f7eb60..0000000000000 --- a/python/ql/src/experimental/Security/CWE-614/InsecureCookie.py +++ /dev/null @@ -1,15 +0,0 @@ -from flask import Flask, request, make_response, Response - - -@app.route("/1") -def true(): - resp = make_response() - resp.set_cookie("name", value="value", secure=True) - return resp - - -@app.route("/2") -def flask_make_response(): - resp = make_response("hello") - resp.headers['Set-Cookie'] = "name=value; Secure;" - return resp diff --git a/python/ql/src/experimental/Security/CWE-614/InsecureCookie.qhelp b/python/ql/src/experimental/Security/CWE-614/InsecureCookie.qhelp deleted file mode 100644 index c76ab17954a05..0000000000000 --- a/python/ql/src/experimental/Security/CWE-614/InsecureCookie.qhelp +++ /dev/null @@ -1,31 +0,0 @@ - - - - -

Setting the 'secure' flag on a cookie to False can cause it to be sent in cleartext. -Setting the 'httponly' flag on a cookie to False may allow attackers access it via JavaScript. -Setting the 'samesite' flag on a cookie to 'None' will make the cookie to be sent in third-party -contexts which may be attacker-controlled.

- - - -

Always set secure to True or add "; Secure;" to the cookie's raw value.

-

Always set httponly to True or add "; HttpOnly;" to the cookie's raw value.

-

Always set samesite to Lax or Strict, or add "; SameSite=Lax;", or -"; Samesite=Strict;" to the cookie's raw header value.

- - - -

This example shows two ways of adding a cookie to a Flask response. The first way uses set_cookie's -secure flag and the second adds the secure flag in the cookie's raw value.

- - - - -
  • Detectify: Cookie lack Secure flag.
    • -
  • PortSwigger: TLS cookie without secure flag set.
    • -     - -     diff --git a/python/ql/src/experimental/Security/CWE-614/InsecureCookie.ql b/python/ql/src/experimental/Security/CWE-614/InsecureCookie.ql deleted file mode 100644 index f34f166ca6fe8..0000000000000 --- a/python/ql/src/experimental/Security/CWE-614/InsecureCookie.ql +++ /dev/null @@ -1,31 +0,0 @@ -/** - * @name Failure to use secure cookies - * @description Insecure cookies may be sent in cleartext, which makes them vulnerable to - * interception. - * @kind problem - * @problem.severity error - * @security-severity 5.0 - * @precision ??? - * @id py/insecure-cookie - * @tags security - * experimental - * external/cwe/cwe-614 - */ - -// TODO: determine precision above -import python -import semmle.python.dataflow.new.DataFlow -import experimental.semmle.python.Concepts -import semmle.python.Concepts - -from Http::Server::CookieWrite cookie, string alert -where - cookie.hasSecureFlag(false) and - alert = "secure" - or - cookie.hasHttpOnlyFlag(false) and - alert = "httponly" - or - cookie.hasSameSiteAttribute(any(Http::Server::CookieWrite::SameSiteNone v)) and - alert = "samesite" -select cookie, "Cookie is added without the '" + alert + "' flag properly set." diff --git a/python/ql/src/experimental/semmle/python/Frameworks.qll b/python/ql/src/experimental/semmle/python/Frameworks.qll index bad485c6a4247..6c9972e42552a 100644 --- a/python/ql/src/experimental/semmle/python/Frameworks.qll +++ b/python/ql/src/experimental/semmle/python/Frameworks.qll @@ -4,7 +4,6 @@ private import experimental.semmle.python.frameworks.AsyncSsh private import experimental.semmle.python.frameworks.Stdlib -private import experimental.semmle.python.frameworks.Flask private import experimental.semmle.python.frameworks.Django private import experimental.semmle.python.frameworks.LDAP private import experimental.semmle.python.frameworks.Netmiko diff --git a/python/ql/src/experimental/semmle/python/frameworks/Django.qll b/python/ql/src/experimental/semmle/python/frameworks/Django.qll index dd3b850a24954..8f3e714268c3a 100644 --- a/python/ql/src/experimental/semmle/python/frameworks/Django.qll +++ b/python/ql/src/experimental/semmle/python/frameworks/Django.qll @@ -87,73 +87,6 @@ private module ExperimentalPrivateDjango { or result = baseClassRef().getReturn().getAMember() } - - /** - * Gets a call to `set_cookie()`. - * - * Given the following example: - * - * ```py - * def django_response(request): - * resp = django.http.HttpResponse() - * resp.set_cookie("name", "value", secure=True, httponly=True, samesite='Lax') - * return resp - * ``` - * - * * `this` would be `resp.set_cookie("name", "value", secure=False, httponly=False, samesite='None')`. - * * `getName()`'s result would be `"name"`. - * * `getValue()`'s result would be `"value"`. - * * `isSecure()` predicate would succeed. - * * `isHttpOnly()` predicate would succeed. - * * `isSameSite()` predicate would succeed. - */ - class DjangoResponseSetCookieCall extends DataFlow::MethodCallNode, - Http::Server::CookieWrite::Range - { - DjangoResponseSetCookieCall() { - this.calls(PrivateDjango::DjangoImpl::DjangoHttp::Response::HttpResponse::instance(), - "set_cookie") - } - - override DataFlow::Node getNameArg() { - result in [this.getArg(0), this.getArgByName("key")] - } - - override DataFlow::Node getValueArg() { - result in [this.getArg(1), this.getArgByName("value")] - } - - override predicate hasSecureFlag(boolean b) { - if - DataFlow::exprNode(any(True t)) - .(DataFlow::LocalSourceNode) - .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("secure")) - then b = true - else b = false - } - - override predicate hasHttpOnlyFlag(boolean b) { - if - DataFlow::exprNode(any(True t)) - .(DataFlow::LocalSourceNode) - .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("httponly")) - then b = true - else b = false - } - - // override predicate hasSameSiteFlag(boolean b) { - // if - // exists(StringLiteral str | - // str.getText() in ["Strict", "Lax"] and - // DataFlow::exprNode(str) - // .(DataFlow::LocalSourceNode) - // .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite")) - // ) - // then b = true - // else b = false - // } - override DataFlow::Node getHeaderArg() { none() } - } } } } diff --git a/python/ql/src/experimental/semmle/python/frameworks/Flask.qll b/python/ql/src/experimental/semmle/python/frameworks/Flask.qll deleted file mode 100644 index 8422b1d949b7d..0000000000000 --- a/python/ql/src/experimental/semmle/python/frameworks/Flask.qll +++ /dev/null @@ -1,72 +0,0 @@ -/** - * Provides classes modeling security-relevant aspects of the `flask` PyPI package. - * See https://flask.palletsprojects.com/en/1.1.x/. - */ - -private import python -private import semmle.python.frameworks.Flask -private import semmle.python.dataflow.new.DataFlow -private import experimental.semmle.python.Concepts -private import semmle.python.Concepts -private import semmle.python.ApiGraphs -private import semmle.python.frameworks.Flask - -module ExperimentalFlask { - /** - * Gets a call to `set_cookie()`. - * - * Given the following example: - * - * ```py - * @app.route("/") - * def false(): - * resp = make_response() - * resp.set_cookie("name", value="value", secure=True, httponly=True, samesite='Lax') - * return resp - * ``` - * - * * `this` would be `resp.set_cookie("name", value="value", secure=False, httponly=False, samesite='None')`. - * * `getName()`'s result would be `"name"`. - * * `getValue()`'s result would be `"value"`. - * * `isSecure()` predicate would succeed. - * * `isHttpOnly()` predicate would succeed. - * * `isSameSite()` predicate would succeed. - */ - class FlaskSetCookieCall extends Http::Server::CookieWrite::Range instanceof Flask::FlaskResponseSetCookieCall - { - override DataFlow::Node getNameArg() { result = this.getNameArg() } - - override DataFlow::Node getValueArg() { result = this.getValueArg() } - - override predicate hasSecureFlag(boolean b) { - if - DataFlow::exprNode(any(True t)) - .(DataFlow::LocalSourceNode) - .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("secure")) - then b = true - else b = false - } - - override predicate hasHttpOnlyFlag(boolean b) { - if - DataFlow::exprNode(any(True t)) - .(DataFlow::LocalSourceNode) - .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("httponly")) - then b = true - else b = false - } - - // override predicate hasSameSiteFlag(boolean b) { - // if - // exists(StringLiteral str | - // str.getText() in ["Strict", "Lax"] and - // DataFlow::exprNode(str) - // .(DataFlow::LocalSourceNode) - // .flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite")) - // ) - // then b = true - // else b = false - // } - override DataFlow::Node getHeaderArg() { none() } - } -} diff --git a/python/ql/src/experimental/semmle/python/security/injection/CookieInjection.qll b/python/ql/src/experimental/semmle/python/security/injection/CookieInjection.qll deleted file mode 100644 index 07a92089ec10d..0000000000000 --- a/python/ql/src/experimental/semmle/python/security/injection/CookieInjection.qll +++ /dev/null @@ -1,44 +0,0 @@ -import python -import experimental.semmle.python.Concepts -import semmle.python.Concepts -import semmle.python.dataflow.new.DataFlow -import semmle.python.dataflow.new.TaintTracking -import semmle.python.dataflow.new.RemoteFlowSources - -class CookieSink extends DataFlow::Node { - string flag; - - CookieSink() { - exists(Http::Server::CookieWrite cookie | - this in [cookie.getNameArg(), cookie.getValueArg(), cookie.getHeaderArg()] and - ( - cookie.hasSecureFlag(false) and - flag = "secure" - or - cookie.hasHttpOnlyFlag(false) and - flag = "httponly" - or - cookie.hasSameSiteAttribute(any(Http::Server::CookieWrite::SameSiteNone v)) and - flag = "samesite" - ) - ) - } - - string getFlag() { result = flag } -} - -/** - * A taint-tracking configuration for detecting Cookie injections. - */ -private module CookieInjectionConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - - predicate isSink(DataFlow::Node sink) { - exists(Http::Server::CookieWrite c | - sink in [c.getNameArg(), c.getValueArg(), c.getHeaderArg()] - ) - } -} - -/** Global taint-tracking for detecting "Cookie injections" vulnerabilities. */ -module CookieInjectionFlow = TaintTracking::Global; diff --git a/python/ql/test/experimental/query-tests/Security/CWE-614/InsecureCookie.expected b/python/ql/test/experimental/query-tests/Security/CWE-614/InsecureCookie.expected deleted file mode 100644 index ba0831d0cb568..0000000000000 --- a/python/ql/test/experimental/query-tests/Security/CWE-614/InsecureCookie.expected +++ /dev/null @@ -1,26 +0,0 @@ -| django_bad.py:6:5:7:52 | ControlFlowNode for Attribute() | Cookie is added without the 'httponly' flag properly set. | -| django_bad.py:6:5:7:52 | ControlFlowNode for Attribute() | Cookie is added without the 'samesite' flag properly set. | -| django_bad.py:6:5:7:52 | ControlFlowNode for Attribute() | Cookie is added without the 'secure' flag properly set. | -| django_bad.py:13:5:13:26 | ControlFlowNode for Subscript | Cookie is added without the 'httponly' flag properly set. | -| django_bad.py:13:5:13:26 | ControlFlowNode for Subscript | Cookie is added without the 'samesite' flag properly set. | -| django_bad.py:13:5:13:26 | ControlFlowNode for Subscript | Cookie is added without the 'secure' flag properly set. | -| django_bad.py:19:5:21:66 | ControlFlowNode for Attribute() | Cookie is added without the 'httponly' flag properly set. | -| django_bad.py:19:5:21:66 | ControlFlowNode for Attribute() | Cookie is added without the 'samesite' flag properly set. | -| django_bad.py:19:5:21:66 | ControlFlowNode for Attribute() | Cookie is added without the 'secure' flag properly set. | -| django_bad.py:27:5:27:26 | ControlFlowNode for Subscript | Cookie is added without the 'samesite' flag properly set. | -| django_good.py:19:5:19:44 | ControlFlowNode for Attribute() | Cookie is added without the 'httponly' flag properly set. | -| django_good.py:19:5:19:44 | ControlFlowNode for Attribute() | Cookie is added without the 'samesite' flag properly set. | -| django_good.py:19:5:19:44 | ControlFlowNode for Attribute() | Cookie is added without the 'secure' flag properly set. | -| flask_bad.py:9:5:10:52 | ControlFlowNode for Attribute() | Cookie is added without the 'httponly' flag properly set. | -| flask_bad.py:9:5:10:52 | ControlFlowNode for Attribute() | Cookie is added without the 'samesite' flag properly set. | -| flask_bad.py:9:5:10:52 | ControlFlowNode for Attribute() | Cookie is added without the 'secure' flag properly set. | -| flask_bad.py:17:5:17:30 | ControlFlowNode for Subscript | Cookie is added without the 'httponly' flag properly set. | -| flask_bad.py:17:5:17:30 | ControlFlowNode for Subscript | Cookie is added without the 'samesite' flag properly set. | -| flask_bad.py:17:5:17:30 | ControlFlowNode for Subscript | Cookie is added without the 'secure' flag properly set. | -| flask_bad.py:24:5:25:52 | ControlFlowNode for Attribute() | Cookie is added without the 'httponly' flag properly set. | -| flask_bad.py:24:5:25:52 | ControlFlowNode for Attribute() | Cookie is added without the 'samesite' flag properly set. | -| flask_bad.py:24:5:25:52 | ControlFlowNode for Attribute() | Cookie is added without the 'secure' flag properly set. | -| flask_bad.py:32:5:32:30 | ControlFlowNode for Subscript | Cookie is added without the 'samesite' flag properly set. | -| flask_good.py:23:5:23:57 | ControlFlowNode for Attribute() | Cookie is added without the 'httponly' flag properly set. | -| flask_good.py:23:5:23:57 | ControlFlowNode for Attribute() | Cookie is added without the 'samesite' flag properly set. | -| flask_good.py:23:5:23:57 | ControlFlowNode for Attribute() | Cookie is added without the 'secure' flag properly set. | diff --git a/python/ql/test/experimental/query-tests/Security/CWE-614/InsecureCookie.qlref b/python/ql/test/experimental/query-tests/Security/CWE-614/InsecureCookie.qlref deleted file mode 100644 index 378d5dcae1a08..0000000000000 --- a/python/ql/test/experimental/query-tests/Security/CWE-614/InsecureCookie.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/Security/CWE-614/InsecureCookie.ql