Merge pull request #16893 from joefarebrother/python-cookie-injectio-promote

Python: Promote cookie injection query from experimental
This commit is contained in:
Joe Farebrother
2024-07-29 10:17:01 +01:00
committed by GitHub
14 changed files with 179 additions and 153 deletions

View File

@@ -0,0 +1,27 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Constructing cookies from user input can allow an attacker to control a user's cookie.
This may lead to a session fixation attack. Additionally, client code may not expect a cookie to contain attacker-controlled data, and fail to sanitize it for common vulnerabilities such as Cross Site Scripting (XSS).
An attacker manipulating the raw cookie header may additionally be able to set cookie attributes such as <code>HttpOnly</code> to insecure values.
</p>
</overview>
<recommendation>
<p>Do not use raw user input to construct cookies.</p>
</recommendation>
<example>
<p>In the following cases, a cookie is constructed for a Flask response using user input. The first uses <code>set_cookie</code>,
and the second sets a cookie's raw value through the <code>set-cookie</code> header.</p>
<sample src="examples/CookieInjection.py" />
</example>
<references>
<li>Wikipedia - <a href="https://en.wikipedia.org/wiki/Session_fixation">Session Fixation</a>.</li>
</references>
</qhelp>

View File

@@ -0,0 +1,20 @@
/**
* @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 warning
* @precision high
* @security-severity 5.0
* @id py/cookie-injection
* @tags security
* external/cwe/cwe-20
*/
import python
import semmle.python.security.dataflow.CookieInjectionQuery
import CookieInjectionFlow::PathGraph
from CookieInjectionFlow::PathNode source, CookieInjectionFlow::PathNode sink
where CookieInjectionFlow::flowPath(source, sink)
select sink.getNode(), source, sink, "Cookie is constructed from a $@.", source.getNode(),
"user-supplied input"

View File

@@ -2,15 +2,15 @@ from flask import request, make_response
@app.route("/1")
def true():
def set_cookie():
resp = make_response()
resp.set_cookie(request.args["name"],
resp.set_cookie(request.args["name"], # BAD: User input is used to set the cookie's name and value
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']};"
def set_cookie_header():
resp = make_response()
resp.headers['Set-Cookie'] = f"{request.args['name']}={request.args['name']};" # BAD: User input is used to set the raw cookie header.
return resp

View File

@@ -0,0 +1,4 @@
---
category: newQuery
---
* The `py/cookie-injection` query, originally contributed to the experimental query pack by @jorgectf, has been promoted to the main query pack. This query finds instances of cookies being constructed from user input.

View File

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

@@ -1,27 +0,0 @@
/**
* @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
* experimental
* 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 CookieInjectionFlow::PathGraph
from CookieInjectionFlow::PathNode source, CookieInjectionFlow::PathNode sink, string insecure
where
CookieInjectionFlow::flowPath(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

@@ -1,41 +0,0 @@
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.
*/
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()])
}
}
/** Global taint-tracking for detecting "Cookie injections" vulnerabilities. */
module CookieInjectionFlow = TaintTracking::Global<CookieInjectionConfig>;