hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-28 02:05:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Add cookie flags to cookie write concept, and alter experimental queries to use them

Browse Source
This commit is contained in:
Joe Farebrother
2024-05-28 09:49:18 +01:00
parent ff8bb2b1f8
commit 6f7b2a2d20
7 changed files with 136 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
python/ql/src/experimental/Security/CWE-614/InsecureCookie.ql
View File

@@ -16,16 +16,17 @@
import python
import semmle.python.dataflow.new.DataFlow
import experimental.semmle.python.Concepts
import semmle.python.Concepts
import experimental.semmle.python.CookieHeader
from Cookie cookie, string alert
from Http::Server::CookieWrite cookie, string alert
where
not cookie.isSecure() and
cookie.getSecureFlag() = false and
alert = "secure"
or
not cookie.isHttpOnly() and
cookie.getHttpOnlyFlag() = false and
alert = "httponly"
or
not cookie.isSameSite() and
cookie.getSameSiteFlag() = false and
alert = "samesite"
select cookie, "Cookie is added without the '" + alert + "' flag properly set."

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

@@ -278,55 +278,6 @@ class CsvWriter extends DataFlow::Node instanceof CsvWriter::Range {
DataFlow::Node getAnInput() { result = super.getAnInput() }
}
/**
* 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 `Cookie::Range` instead.
*/
class Cookie extends Http::Server::CookieWrite instanceof Cookie::Range {
/**
* Holds if this cookie is secure.
*/
predicate isSecure() { super.isSecure() }
/**
* Holds if this cookie is HttpOnly.
*/
predicate isHttpOnly() { super.isHttpOnly() }
/**
* Holds if the cookie is SameSite
*/
predicate isSameSite() { super.isSameSite() }
}
/** Provides a class for modeling new cookie writes on HTTP responses. */
module Cookie {
/**
* A data-flow node that sets a cookie in an HTTP response.
*
* Extend this class to model new APIs. If you want to refine existing API models,
* extend `Cookie` instead.
*/
abstract class Range extends Http::Server::CookieWrite::Range {
/**
* Holds if this cookie is secure.
*/
abstract predicate isSecure();
/**
* Holds if this cookie is HttpOnly.
*/
abstract predicate isHttpOnly();
/**
* Holds if the cookie is SameSite.
*/
abstract predicate isSameSite();
}
}
/** Provides classes for modeling JWT encoding-related APIs. */
module JwtEncoding {
/**

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

@@ -27,7 +27,8 @@ import semmle.python.Concepts
* * `isSameSite()` predicate would fail.
* * `getName()` and `getValue()` results would be `"name=value; Secure;"`.
*/
class CookieHeader extends Cookie::Range instanceof Http::Server::ResponseHeaderWrite {
class CookieHeader extends Http::Server::CookieWrite::Range instanceof Http::Server::ResponseHeaderWrite
{
CookieHeader() {
exists(StringLiteral str |
str.getText() = "Set-Cookie" and
@@ -37,31 +38,40 @@ class CookieHeader extends Cookie::Range instanceof Http::Server::ResponseHeader
)
}
override predicate isSecure() {
exists(StringLiteral str |
str.getText().regexpMatch(".*; *Secure;.*") and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(Http::Server::ResponseHeaderWrite).getValueArg())
)
override boolean getSecureFlag() {
if
exists(StringLiteral str |
str.getText().regexpMatch(".*; *Secure;.*") and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(Http::Server::ResponseHeaderWrite).getValueArg())
)
then result = true
else result = false
}
override predicate isHttpOnly() {
exists(StringLiteral str |
str.getText().regexpMatch(".*; *HttpOnly;.*") and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(Http::Server::ResponseHeaderWrite).getValueArg())
)
override boolean getHttpOnlyFlag() {
if
exists(StringLiteral str |
str.getText().regexpMatch(".*; *HttpOnly;.*") and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(Http::Server::ResponseHeaderWrite).getValueArg())
)
then result = true
else result = false
}
override predicate isSameSite() {
exists(StringLiteral str |
str.getText().regexpMatch(".*; *SameSite=(Strict|Lax);.*") and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(Http::Server::ResponseHeaderWrite).getValueArg())
)
override boolean getSameSiteFlag() {
if
exists(StringLiteral str |
str.getText().regexpMatch(".*; *SameSite=(Strict|Lax);.*") and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(Http::Server::ResponseHeaderWrite).getValueArg())
)
then result = true
else result = false
}
override DataFlow::Node getNameArg() {

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

@@ -107,7 +107,9 @@ private module ExperimentalPrivateDjango {
* * `isHttpOnly()` predicate would succeed.
* * `isSameSite()` predicate would succeed.
*/
class DjangoResponseSetCookieCall extends DataFlow::MethodCallNode, Cookie::Range {
class DjangoResponseSetCookieCall extends DataFlow::MethodCallNode,
Http::Server::CookieWrite::Range
{
DjangoResponseSetCookieCall() {
this.calls(PrivateDjango::DjangoImpl::DjangoHttp::Response::HttpResponse::instance(),
"set_cookie")
@@ -121,25 +123,34 @@ private module ExperimentalPrivateDjango {
result in [this.getArg(1), this.getArgByName("value")]
}
override predicate isSecure() {
DataFlow::exprNode(any(True t))
.(DataFlow::LocalSourceNode)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("secure"))
}
override predicate isHttpOnly() {
DataFlow::exprNode(any(True t))
.(DataFlow::LocalSourceNode)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("httponly"))
}
override predicate isSameSite() {
exists(StringLiteral str |
str.getText() in ["Strict", "Lax"] and
DataFlow::exprNode(str)
override boolean getSecureFlag() {
if
DataFlow::exprNode(any(True t))
.(DataFlow::LocalSourceNode)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite"))
)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("secure"))
then result = true
else result = false
}
override boolean getHttpOnlyFlag() {
if
DataFlow::exprNode(any(True t))
.(DataFlow::LocalSourceNode)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("httponly"))
then result = true
else result = false
}
override boolean getSameSiteFlag() {
if
exists(StringLiteral str |
str.getText() in ["Strict", "Lax"] and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite"))
)
then result = true
else result = false
}
override DataFlow::Node getHeaderArg() { none() }

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

@@ -7,6 +7,7 @@ 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
@@ -31,30 +32,40 @@ module ExperimentalFlask {
* * `isHttpOnly()` predicate would succeed.
* * `isSameSite()` predicate would succeed.
*/
class FlaskSetCookieCall extends Cookie::Range instanceof Flask::FlaskResponseSetCookieCall {
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 isSecure() {
DataFlow::exprNode(any(True t))
.(DataFlow::LocalSourceNode)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("secure"))
}
override predicate isHttpOnly() {
DataFlow::exprNode(any(True t))
.(DataFlow::LocalSourceNode)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("httponly"))
}
override predicate isSameSite() {
exists(StringLiteral str |
str.getText() in ["Strict", "Lax"] and
DataFlow::exprNode(str)
override boolean getSecureFlag() {
if
DataFlow::exprNode(any(True t))
.(DataFlow::LocalSourceNode)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite"))
)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("secure"))
then result = true
else result = false
}
override boolean getHttpOnlyFlag() {
if
DataFlow::exprNode(any(True t))
.(DataFlow::LocalSourceNode)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("httponly"))
then result = true
else result = false
}
override boolean getSameSiteFlag() {
if
exists(StringLiteral str |
str.getText() in ["Strict", "Lax"] and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite"))
)
then result = true
else result = false
}
override DataFlow::Node getHeaderArg() { none() }

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

@@ -1,5 +1,6 @@
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
@@ -8,16 +9,16 @@ class CookieSink extends DataFlow::Node {
string flag;
CookieSink() {
exists(Cookie cookie |
this in [cookie.getNameArg(), cookie.getValueArg()] and
exists(Http::Server::CookieWrite cookie |
this in [cookie.getNameArg(), cookie.getValueArg(), cookie.getHeaderArg()] and
(
not cookie.isSecure() and
cookie.getSecureFlag() = false and
flag = "secure"
or
not cookie.isHttpOnly() and
cookie.getHttpOnlyFlag() = false and
flag = "httponly"
or
not cookie.isSameSite() and
cookie.getSameSiteFlag() = false and
flag = "samesite"
)
)
@@ -33,7 +34,9 @@ private module CookieInjectionConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
predicate isSink(DataFlow::Node sink) {
exists(Cookie c | sink in [c.getNameArg(), c.getValueArg()])
exists(Http::Server::CookieWrite c |
sink in [c.getNameArg(), c.getValueArg(), c.getHeaderArg()]
)
}
}