Merge branch 'main' into promote-pam

This commit is contained in:
Rasmus Wriedt Larsen
2022-05-18 10:35:39 +02:00
1485 changed files with 118149 additions and 19485 deletions

View File

@@ -1,3 +1,11 @@
## 0.1.2
### New Queries
* "XML external entity expansion" (`py/xxe`). Results will appear by default. This query was based on [an experimental query by @jorgectf](https://github.com/github/codeql/pull/6112).
* "XML internal entity expansion" (`py/xml-bomb`). Results will appear by default. This query was based on [an experimental query by @jorgectf](https://github.com/github/codeql/pull/6112).
* The query "CSRF protection weakened or disabled" (`py/csrf-protection-disabled`) has been implemented. Its results will now appear by default.
## 0.1.1
## 0.1.0

View File

@@ -81,11 +81,11 @@ class ExcludeTarFilePy extends Sanitizer {
/* Any call to an extractall method */
class ExtractAllSink extends TaintSink {
CallNode call;
ExtractAllSink() {
this = call.getFunction().(AttrNode).getObject("extractall") and
count(call.getAnArg()) = 0
exists(CallNode call |
this = call.getFunction().(AttrNode).getObject("extractall") and
not exists(call.getAnArg())
)
}
override predicate sinks(TaintKind kind) { kind instanceof OpenTarFile }

View File

@@ -17,7 +17,7 @@ import semmle.python.dataflow.new.TaintTracking
API::Node libPam() {
exists(API::CallNode findLibCall, API::CallNode cdllCall |
findLibCall = API::moduleImport("ctypes.util").getMember("find_library").getACall() and
findLibCall = API::moduleImport("ctypes").getMember("util").getMember("find_library").getACall() and
findLibCall.getParameter(0).getAValueReachingRhs().asExpr().(StrConst).getText() = "pam" and
cdllCall = API::moduleImport("ctypes").getMember("CDLL").getACall() and
cdllCall.getParameter(0).getAValueReachingRhs() = findLibCall

View File

@@ -194,7 +194,7 @@ predicate function_object_consistency(string clsname, string problem, string wha
exists(FunctionObject func | clsname = func.getAQlClass() |
what = func.getName() and
(
count(func.descriptiveString()) = 0 and problem = "no descriptiveString()"
not exists(func.descriptiveString()) and problem = "no descriptiveString()"
or
exists(int c | c = strictcount(func.descriptiveString()) and c > 1 |
problem = c + "descriptiveString()s"

View File

@@ -1,4 +0,0 @@
---
category: newQuery
---
* The query "CSRF protection weakened or disabled" (`py/csrf-protection-disabled`) has been implemented. Its results will now appear by default.

View File

@@ -1,5 +1,7 @@
---
category: newQuery
---
## 0.1.2
### New Queries
* "XML external entity expansion" (`py/xxe`). Results will appear by default. This query was based on [an experimental query by @jorgectf](https://github.com/github/codeql/pull/6112).
* "XML internal entity expansion" (`py/xml-bomb`). Results will appear by default. This query was based on [an experimental query by @jorgectf](https://github.com/github/codeql/pull/6112).
* The query "CSRF protection weakened or disabled" (`py/csrf-protection-disabled`) has been implemented. Its results will now appear by default.

View File

@@ -1,2 +1,2 @@
---
lastReleaseVersion: 0.1.1
lastReleaseVersion: 0.1.2

View File

@@ -0,0 +1,16 @@
from flask import request, make_response
@app.route("/1")
def true():
resp = make_response()
resp.set_cookie(request.args["name"],
value=request.args["name"])
return resp
@app.route("/2")
def flask_make_response():
resp = make_response("hello")
resp.headers['Set-Cookie'] = f"{request.args['name']}={request.args['name']};"
return resp

View File

@@ -0,0 +1,28 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Constructing cookies from user input may allow an attacker to perform a Cookie Poisoning attack.
It is possible, however, to perform other parameter-like attacks through cookie poisoning techniques,
such as SQL Injection, Directory Traversal, or Stealth Commanding, etc. Additionally,
cookie injection may relate to attempts to perform Access of Administrative Interface.
</p>
</overview>
<recommendation>
<p>Do not use raw user input to construct cookies.</p>
</recommendation>
<example>
<p>This example shows two ways of adding a cookie to a Flask response. The first way uses <code>set_cookie</code>'s
and the second sets a cookie's raw value through a header, both using user-supplied input.</p>
<sample src="CookieInjection.py" />
</example>
<references>
<li>Imperva: <a href="https://docs.imperva.com/bundle/on-premises-knowledgebase-reference-guide/page/cookie_injection.htm">Cookie injection</a>.</li>
</references>
</qhelp>

View File

@@ -0,0 +1,28 @@
/**
* @name Construction of a cookie using user-supplied input.
* @description Constructing cookies from user input may allow an attacker to perform a Cookie Poisoning attack.
* @kind path-problem
* @problem.severity error
* @id py/cookie-injection
* @tags security
* external/cwe/cwe-614
*/
// determine precision above
import python
import semmle.python.dataflow.new.DataFlow
import experimental.semmle.python.Concepts
import experimental.semmle.python.CookieHeader
import experimental.semmle.python.security.injection.CookieInjection
import DataFlow::PathGraph
from
CookieInjectionFlowConfig config, DataFlow::PathNode source, DataFlow::PathNode sink,
string insecure
where
config.hasFlowPath(source, sink) and
if exists(sink.getNode().(CookieSink))
then insecure = ",and its " + sink.getNode().(CookieSink).getFlag() + " flag is not properly set."
else insecure = "."
select sink.getNode(), source, sink, "Cookie is constructed from a $@" + insecure, source.getNode(),
"user-supplied input"

View File

@@ -0,0 +1,15 @@
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

View File

@@ -0,0 +1,31 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Setting the 'secure' flag on a cookie to <code>False</code> can cause it to be sent in cleartext.
Setting the 'httponly' flag on a cookie to <code>False</code> may allow attackers access it via JavaScript.
Setting the 'samesite' flag on a cookie to <code>'None'</code> will make the cookie to be sent in third-party
contexts which may be attacker-controlled.</p>
</overview>
<recommendation>
<p>Always set <code>secure</code> to <code>True</code> or add "; Secure;" to the cookie's raw value.</p>
<p>Always set <code>httponly</code> to <code>True</code> or add "; HttpOnly;" to the cookie's raw value.</p>
<p>Always set <code>samesite</code> to <code>Lax</code> or <code>Strict</code>, or add "; SameSite=Lax;", or
"; Samesite=Strict;" to the cookie's raw header value.</p>
</recommendation>
<example>
<p>This example shows two ways of adding a cookie to a Flask response. The first way uses <code>set_cookie</code>'s
secure flag and the second adds the secure flag in the cookie's raw value.</p>
<sample src="InsecureCookie.py" />
</example>
<references>
<li>Detectify: <a href="https://support.detectify.com/support/solutions/articles/48001048982-cookie-lack-secure-flag">Cookie lack Secure flag</a>.</li>
<li>PortSwigger: <a href="https://portswigger.net/kb/issues/00500200_tls-cookie-without-secure-flag-set">TLS cookie without secure flag set</a>.</li>
</references>
</qhelp>

View File

@@ -0,0 +1,30 @@
/**
* @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
* external/cwe/cwe-614
*/
// TODO: determine precision above
import python
import semmle.python.dataflow.new.DataFlow
import experimental.semmle.python.Concepts
import experimental.semmle.python.CookieHeader
from Cookie cookie, string alert
where
not cookie.isSecure() and
alert = "secure"
or
not cookie.isHttpOnly() and
alert = "httponly"
or
not cookie.isSameSite() and
alert = "samesite"
select cookie, "Cookie is added without the '" + alert + "' flag properly set."

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 copying file related APIs. */
module CopyFile {
@@ -370,6 +371,55 @@ class HeaderDeclaration extends DataFlow::Node {
DataFlow::Node getValueArg() { result = range.getValueArg() }
}
/**
* 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 {
/**

View File

@@ -0,0 +1,72 @@
/**
* 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
/**
* Gets a header setting a cookie.
*
* Given the following example:
*
* ```py
* @app.route("/")
* def flask_make_response():
* resp = make_response("")
* resp.headers['Set-Cookie'] = "name=value; Secure;"
* return resp
* ```
*
* * `this` would be `resp.headers['Set-Cookie'] = "name=value; Secure;"`.
* * `isSecure()` predicate would succeed.
* * `isHttpOnly()` predicate would fail.
* * `isSameSite()` predicate would fail.
* * `getName()` and `getValue()` results would be `"name=value; Secure;"`.
*/
class CookieHeader extends Cookie::Range instanceof HeaderDeclaration {
CookieHeader() {
this instanceof HeaderDeclaration and
exists(StrConst str |
str.getText() = "Set-Cookie" and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(HeaderDeclaration).getNameArg())
)
}
override predicate isSecure() {
exists(StrConst str |
str.getText().regexpMatch(".*; *Secure;.*") and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(HeaderDeclaration).getValueArg())
)
}
override predicate isHttpOnly() {
exists(StrConst str |
str.getText().regexpMatch(".*; *HttpOnly;.*") and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(HeaderDeclaration).getValueArg())
)
}
override predicate isSameSite() {
exists(StrConst str |
str.getText().regexpMatch(".*; *SameSite=(Strict|Lax);.*") and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(HeaderDeclaration).getValueArg())
)
}
override DataFlow::Node getNameArg() { result = this.(HeaderDeclaration).getValueArg() }
override DataFlow::Node getValueArg() { result = this.(HeaderDeclaration).getValueArg() }
override DataFlow::Node getHeaderArg() { none() }
}

View File

@@ -9,6 +9,7 @@ private import semmle.python.dataflow.new.DataFlow
private import experimental.semmle.python.Concepts
private import semmle.python.ApiGraphs
import semmle.python.dataflow.new.RemoteFlowSources
private import semmle.python.Concepts
private module ExperimentalPrivateDjango {
private module DjangoMod {
@@ -32,22 +33,64 @@ private module ExperimentalPrivateDjango {
module Response {
module HttpResponse {
API::Node baseClassRef() {
result = response().getMember("HttpResponse").getReturn()
result = response().getMember("HttpResponse")
or
// Handle `django.http.HttpResponse` alias
result = http().getMember("HttpResponse").getReturn()
result = http().getMember("HttpResponse")
}
/** Gets a reference to the `django.http.response.HttpResponse` class. */
API::Node classRef() { result = baseClassRef().getASubclass*() }
/**
* A source of instances of `django.http.response.HttpResponse`, extend this class to model new instances.
*
* This can include instantiations of the class, return values from function
* calls, or a special parameter that will be set when functions are called by an external
* library.
*
* Use the predicate `HttpResponse::instance()` to get references to instances of `django.http.response.HttpResponse`.
*/
abstract class InstanceSource extends HTTP::Server::HttpResponse::Range, DataFlow::Node {
}
/** A direct instantiation of `django.http.response.HttpResponse`. */
private class ClassInstantiation extends InstanceSource, DataFlow::CallCfgNode {
ClassInstantiation() { this = classRef().getACall() }
override DataFlow::Node getBody() {
result in [this.getArg(0), this.getArgByName("content")]
}
// How to support the `headers` argument here?
override DataFlow::Node getMimetypeOrContentTypeArg() {
result in [this.getArg(1), this.getArgByName("content_type")]
}
override string getMimetypeDefault() { result = "text/html" }
}
/** Gets a reference to an instance of `django.http.response.HttpResponse`. */
private DataFlow::TypeTrackingNode instance(DataFlow::TypeTracker t) {
t.start() and
result instanceof InstanceSource
or
exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t))
}
/** Gets a reference to an instance of `django.http.response.HttpResponse`. */
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
/** Gets a reference to a header instance. */
private DataFlow::LocalSourceNode headerInstance(DataFlow::TypeTracker t) {
t.start() and
(
exists(SubscriptNode subscript |
subscript.getObject() = baseClassRef().getAUse().asCfgNode() and
subscript.getObject() = baseClassRef().getReturn().getAUse().asCfgNode() and
result.asCfgNode() = subscript
)
or
result.(DataFlow::AttrRead).getObject() = baseClassRef().getAUse()
result.(DataFlow::AttrRead).getObject() = baseClassRef().getReturn().getAUse()
)
or
exists(DataFlow::TypeTracker t2 | result = headerInstance(t2).track(t2, t))
@@ -86,6 +129,63 @@ private module ExperimentalPrivateDjango {
override DataFlow::Node getValueArg() { result = headerInput }
}
/**
* 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, Cookie::Range {
DjangoResponseSetCookieCall() {
this.calls(PrivateDjango::DjangoImpl::Http::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 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(StrConst str |
str.getText() in ["Strict", "Lax"] and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite"))
)
}
override DataFlow::Node getHeaderArg() { none() }
}
}
}
}

View File

@@ -8,6 +8,7 @@ private import semmle.python.frameworks.Flask
private import semmle.python.dataflow.new.DataFlow
private import experimental.semmle.python.Concepts
private import semmle.python.ApiGraphs
private import semmle.python.frameworks.Flask
module ExperimentalFlask {
/**
@@ -66,10 +67,62 @@ module ExperimentalFlask {
private class FlaskResponse extends DataFlow::CallCfgNode, HeaderDeclaration::Range {
KeyValuePair item;
FlaskResponse() { this = Flask::Response::classRef().getACall() }
FlaskResponse() {
this = Flask::Response::classRef().getACall() and
item = this.getArg(_).asExpr().(Dict).getAnItem()
}
override DataFlow::Node getNameArg() { result.asExpr() = item.getKey() }
override DataFlow::Node getValueArg() { result.asExpr() = item.getValue() }
}
/**
* 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 Cookie::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(StrConst str |
str.getText() in ["Strict", "Lax"] and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(DataFlow::CallCfgNode).getArgByName("samesite"))
)
}
override DataFlow::Node getHeaderArg() { none() }
}
}

View File

@@ -210,10 +210,13 @@ private module NoSql {
*/
private class BsonObjectIdCall extends DataFlow::CallCfgNode, NoSqlSanitizer::Range {
BsonObjectIdCall() {
this =
API::moduleImport(["bson", "bson.objectid", "bson.json_util"])
.getMember("ObjectId")
.getACall()
exists(API::Node mod |
mod = API::moduleImport("bson")
or
mod = API::moduleImport("bson").getMember(["objectid", "json_util"])
|
this = mod.getMember("ObjectId").getACall()
)
}
override DataFlow::Node getAnInput() { result = this.getArg(0) }

View File

@@ -0,0 +1,40 @@
import python
import experimental.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(Cookie cookie |
this in [cookie.getNameArg(), cookie.getValueArg()] and
(
not cookie.isSecure() and
flag = "secure"
or
not cookie.isHttpOnly() and
flag = "httponly"
or
not cookie.isSameSite() and
flag = "samesite"
)
)
}
string getFlag() { result = flag }
}
/**
* A taint-tracking configuration for detecting Cookie injections.
*/
class CookieInjectionFlowConfig extends TaintTracking::Configuration {
CookieInjectionFlowConfig() { this = "CookieInjectionFlowConfig" }
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
override predicate isSink(DataFlow::Node sink) {
exists(Cookie c | sink in [c.getNameArg(), c.getValueArg()])
}
}

View File

@@ -1,5 +1,5 @@
name: codeql/python-queries
version: 0.1.2-dev
version: 0.1.3-dev
groups:
- python
- queries