hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-21 11:16:30 +01:00
Code Issues Packages Projects Releases Wiki Activity

Make Cookie concept extend HTTP::Server::CookieWrite

Browse Source
This commit is contained in:
jorgectf
2021-11-16 13:54:25 +01:00
parent 83e3de1fed
commit e7d649f36d
6 changed files with 24 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
python/ql/src/experimental/semmle/python/Concepts.qll
View File

@@ -13,6 +13,7 @@ private import semmle.python.dataflow.new.DataFlow
private import semmle.python.dataflow.new.RemoteFlowSources
private import semmle.python.dataflow.new.TaintTracking
private import experimental.semmle.python.Frameworks
private import semmle.python.Concepts
/** Provides classes for modeling log related APIs. */
module LogOutput {
@@ -303,35 +304,21 @@ class HeaderDeclaration extends DataFlow::Node {
* Extend this class to refine existing API models. If you want to model new APIs,
* extend `Cookie::Range` instead.
*/
class Cookie extends DataFlow::Node {
Cookie::Range range;
Cookie() { this = range }
class Cookie extends HTTP::Server::CookieWrite instanceof Cookie::Range {
/**
* Holds if this cookie is secure.
*/
predicate isSecure() { range.isSecure() }
predicate isSecure() { super.isSecure() }
/**
* Holds if this cookie is HttpOnly.
*/
predicate isHttpOnly() { range.isHttpOnly() }
predicate isHttpOnly() { super.isHttpOnly() }
/**
* Holds if the cookie is SameSite
*/
predicate isSameSite() { range.isSameSite() }
/**
* Gets the argument containing the header name.
*/
DataFlow::Node getName() { result = range.getName() }
/**
* Gets the argument containing the header value.
*/
DataFlow::Node getValue() { result = range.getValue() }
predicate isSameSite() { super.isSameSite() }
}
/** Provides a class for modeling new cookie writes on HTTP responses. */
@@ -342,7 +329,7 @@ module Cookie {
* Extend this class to model new APIs. If you want to refine existing API models,
* extend `Cookie` instead.
*/
abstract class Range extends DataFlow::Node {
abstract class Range extends HTTP::Server::CookieWrite::Range {
/**
* Holds if this cookie is secure.
*/
@@ -357,15 +344,5 @@ module Cookie {
* Holds if the cookie is SameSite.
*/
abstract predicate isSameSite();
/**
* Gets the argument containing the header name.
*/
abstract DataFlow::Node getName();
/**
* Gets the argument containing the header value.
*/
abstract DataFlow::Node getValue();
}
}

8
python/ql/src/experimental/semmle/python/CookieHeader.qll
View File

@@ -26,7 +26,7 @@ import experimental.semmle.python.Concepts
* * `isSameSite()` predicate would fail.
* * `getName()` and `getValue()` results would be `"name=value; Secure;"`.
*/
class CookieHeader extends HeaderDeclaration, Cookie::Range {
class CookieHeader extends Cookie::Range instanceof HeaderDeclaration {
CookieHeader() {
this instanceof HeaderDeclaration and
this.(HeaderDeclaration).getNameArg().asExpr().(Str_).getS() = "Set-Cookie"
@@ -49,7 +49,9 @@ class CookieHeader extends HeaderDeclaration, Cookie::Range {
.regexpMatch(".*; *SameSite=(Strict|Lax);.*")
}
override DataFlow::Node getName() { result = this.(HeaderDeclaration).getValueArg() }
override DataFlow::Node getNameArg() { result = this.(HeaderDeclaration).getValueArg() }
override DataFlow::Node getValue() { result = this.(HeaderDeclaration).getValueArg() }
override DataFlow::Node getValueArg() { result = this.(HeaderDeclaration).getValueArg() }
override DataFlow::Node getHeaderArg() { none() }
}

6
python/ql/src/experimental/semmle/python/frameworks/Django.qll
View File

@@ -109,9 +109,9 @@ private module PrivateDjango {
class DjangoSetCookieCall extends DataFlow::CallCfgNode, Cookie::Range {
DjangoSetCookieCall() { this = baseClassRef().getMember("set_cookie").getACall() }
override DataFlow::Node getName() { result = this.getArg(0) }
override DataFlow::Node getNameArg() { result = this.getArg(0) }
override DataFlow::Node getValue() { result = this.getArgByName("value") }
override DataFlow::Node getValueArg() { result = this.getArgByName("value") }
override predicate isSecure() {
DataFlow::exprNode(any(True t))
@@ -128,6 +128,8 @@ private module PrivateDjango {
override predicate isSameSite() {
this.getArgByName("samesite").asExpr().(Str_).getS() in ["Strict", "Lax"]
}
override DataFlow::Node getHeaderArg() { none() }
}
}
}

6
python/ql/src/experimental/semmle/python/frameworks/Flask.qll
View File

@@ -111,9 +111,9 @@ module ExperimentalFlask {
.getACall()
}
override DataFlow::Node getName() { result = this.getArg(0) }
override DataFlow::Node getNameArg() { result = this.getArg(0) }
override DataFlow::Node getValue() { result = this.getArgByName("value") }
override DataFlow::Node getValueArg() { result = this.getArgByName("value") }
override predicate isSecure() {
DataFlow::exprNode(any(True t))
@@ -130,5 +130,7 @@ module ExperimentalFlask {
override predicate isSameSite() {
this.getArgByName("samesite").asExpr().(Str_).getS() in ["Strict", "Lax"]
}
override DataFlow::Node getHeaderArg() { none() }
}
}

4
python/ql/src/experimental/semmle/python/security/injection/CookieInjection.qll
View File

@@ -9,7 +9,7 @@ class CookieSink extends DataFlow::Node {
CookieSink() {
exists(Cookie cookie |
this in [cookie.getName(), cookie.getValue()] and
this in [cookie.getNameArg(), cookie.getValueArg()] and
(
not cookie.isSecure() and
flag = "secure"
@@ -35,6 +35,6 @@ class CookieInjectionFlowConfig extends TaintTracking::Configuration {
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
override predicate isSink(DataFlow::Node sink) {
exists(Cookie c | sink in [c.getName(), c.getValue()])
exists(Cookie c | sink in [c.getNameArg(), c.getValueArg()])
}
}

3
python/ql/test/experimental/query-tests/Security/CWE-614/CookieInjection.expected
View File

@@ -1,6 +1,9 @@
| django_bad.py:19:21:19:55 | ControlFlowNode for Attribute() | Cookie is constructed from a | django_bad.py:19:21:19:55 | ControlFlowNode for Attribute() | user-supplied input | and its httponly flag is not properly set |
| django_bad.py:19:21:19:55 | ControlFlowNode for Attribute() | Cookie is constructed from a | django_bad.py:19:21:19:55 | ControlFlowNode for Attribute() | user-supplied input | and its samesite flag is not properly set |
| django_bad.py:19:21:19:55 | ControlFlowNode for Attribute() | Cookie is constructed from a | django_bad.py:19:21:19:55 | ControlFlowNode for Attribute() | user-supplied input | and its secure flag is not properly set |
| django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | Cookie is constructed from a | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | user-supplied input | and its httponly flag is not properly set |
| django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | Cookie is constructed from a | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | user-supplied input | and its samesite flag is not properly set |
| django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | Cookie is constructed from a | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | user-supplied input | and its secure flag is not properly set |
| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | user-supplied input | and its httponly flag is not properly set |
| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | user-supplied input | and its samesite flag is not properly set |
| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | user-supplied input | and its secure flag is not properly set |