Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python: Promote cookie injection query from experimental #16893

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* Provides default sources, sinks and sanitizers for detecting
* "cookie injection"
* vulnerabilities, as well as extension points for adding your own.
*/

private import python
private import semmle.python.dataflow.new.DataFlow
private import semmle.python.Concepts
private import semmle.python.dataflow.new.RemoteFlowSources

/**
* Provides default sources, sinks and sanitizers for detecting
* "cookie injection"
* vulnerabilities, as well as extension points for adding your own.
*/
module CookieInjection {
/**
* A data flow source for "cookie injection" vulnerabilities.
*/
abstract class Source extends DataFlow::Node { }

/**
* A data flow sink for "cookie injection" vulnerabilities.
*/
abstract class Sink extends DataFlow::Node { }

/**
* A sanitizer for "cookie injection" vulnerabilities.
*/
abstract class Sanitizer extends DataFlow::Node { }

/**
* A source of remote user input, considered as a flow source.
*/
class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }

/**
* A write to a cookie, considered as a sink.
*/
class CookieWriteSink extends Sink {
CookieWriteSink() {
exists(Http::Server::CookieWrite cw |
this = [cw.getNameArg(), cw.getValueArg(), cw.getHeaderArg()]
)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Provides a taint-tracking configuration for detecting "cookie injection" vulnerabilities.
*
* Note, for performance reasons: only import this file if
* `CookieInjectionFlow` is needed, otherwise
* `CookieInjectionCustomizations` should be imported instead.
*/

private import python
import semmle.python.dataflow.new.DataFlow
import semmle.python.dataflow.new.TaintTracking
import CookieInjectionCustomizations::CookieInjection

/**
* A taint-tracking configuration for detecting "cookie injection" vulnerabilities.
*/
module CookieInjectionConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof Source }

predicate isSink(DataFlow::Node sink) { sink instanceof Sink }

predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
}

/** Global taint-tracking for detecting "cookie injection" vulnerabilities. */
module CookieInjectionFlow = TaintTracking::Global<CookieInjectionConfig>;
27 changes: 27 additions & 0 deletions python/ql/src/Security/CWE-020/CookieInjection.qhelp
Original file line number Diff line number Diff line change
@@ -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>
20 changes: 20 additions & 0 deletions python/ql/src/Security/CWE-020/CookieInjection.ql
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@


@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
4 changes: 4 additions & 0 deletions python/ql/src/change-notes/2024-07-19-cookie-injection.md
Original file line number Diff line number Diff line change
@@ -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.

This file was deleted.

27 changes: 0 additions & 27 deletions python/ql/src/experimental/Security/CWE-614/CookieInjection.ql

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Loading
Loading