Skip to content

Commit 8c277bd

Browse files
joefarebrotherjoefarebrother
authored
Merge pull request #20494 from joefarebrother/python-insecure-cookie-split
Python: Split Insecure Cookie query into multiple queries
2 parents 672977a + cb7b1ef commit 8c277bd

File tree

24 files changed

+222
-68
lines changed

24 files changed

+222
-68
lines changed

‎python/ql/integration-tests/query-suite/python-code-scanning.qls.expected‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@ ql/python/ql/src/Security/CWE-079/ReflectedXss.ql
1313
ql/python/ql/src/Security/CWE-089/SqlInjection.ql
1414
ql/python/ql/src/Security/CWE-090/LdapInjection.ql
1515
ql/python/ql/src/Security/CWE-094/CodeInjection.ql
16+
ql/python/ql/src/Security/CWE-1004/NonHttpOnlyCookie.ql
1617
ql/python/ql/src/Security/CWE-113/HeaderInjection.ql
1718
ql/python/ql/src/Security/CWE-116/BadTagFilter.ql
19+
ql/python/ql/src/Security/CWE-1275/SameSiteNoneCookie.ql
1820
ql/python/ql/src/Security/CWE-209/StackTraceExposure.ql
1921
ql/python/ql/src/Security/CWE-215/FlaskDebug.ql
2022
ql/python/ql/src/Security/CWE-285/PamAuthorization.ql

‎python/ql/integration-tests/query-suite/python-security-and-quality.qls.expected‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,11 @@ ql/python/ql/src/Security/CWE-079/ReflectedXss.ql
106106
ql/python/ql/src/Security/CWE-089/SqlInjection.ql
107107
ql/python/ql/src/Security/CWE-090/LdapInjection.ql
108108
ql/python/ql/src/Security/CWE-094/CodeInjection.ql
109+
ql/python/ql/src/Security/CWE-1004/NonHttpOnlyCookie.ql
109110
ql/python/ql/src/Security/CWE-113/HeaderInjection.ql
110111
ql/python/ql/src/Security/CWE-116/BadTagFilter.ql
111112
ql/python/ql/src/Security/CWE-117/LogInjection.ql
113+
ql/python/ql/src/Security/CWE-1275/SameSiteNoneCookie.ql
112114
ql/python/ql/src/Security/CWE-209/StackTraceExposure.ql
113115
ql/python/ql/src/Security/CWE-215/FlaskDebug.ql
114116
ql/python/ql/src/Security/CWE-285/PamAuthorization.ql

‎python/ql/integration-tests/query-suite/python-security-extended.qls.expected‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@ ql/python/ql/src/Security/CWE-079/ReflectedXss.ql
1616
ql/python/ql/src/Security/CWE-089/SqlInjection.ql
1717
ql/python/ql/src/Security/CWE-090/LdapInjection.ql
1818
ql/python/ql/src/Security/CWE-094/CodeInjection.ql
19+
ql/python/ql/src/Security/CWE-1004/NonHttpOnlyCookie.ql
1920
ql/python/ql/src/Security/CWE-113/HeaderInjection.ql
2021
ql/python/ql/src/Security/CWE-116/BadTagFilter.ql
2122
ql/python/ql/src/Security/CWE-117/LogInjection.ql
23+
ql/python/ql/src/Security/CWE-1275/SameSiteNoneCookie.ql
2224
ql/python/ql/src/Security/CWE-209/StackTraceExposure.ql
2325
ql/python/ql/src/Security/CWE-215/FlaskDebug.ql
2426
ql/python/ql/src/Security/CWE-285/PamAuthorization.ql

‎python/ql/lib/semmle/python/Concepts.qll‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ private import semmle.python.dataflow.new.TaintTracking
1212
private import semmle.python.Files
1313
private import semmle.python.Frameworks
1414
private import semmle.python.security.internal.EncryptionKeySizes
15+
private import semmle.python.dataflow.new.SensitiveDataSources
1516
private import codeql.threatmodels.ThreatModels
1617
private import codeql.concepts.ConceptsShared
1718

@@ -1290,6 +1291,18 @@ module Http {
12901291
*/
12911292
DataFlow::Node getValueArg() { result = super.getValueArg() }
12921293

1294+
/** Holds if the name of this cookie indicates it may contain sensitive information. */
1295+
predicate isSensitive() {
1296+
exists(DataFlow::Node name |
1297+
name = [this.getNameArg(), this.getHeaderArg()] and
1298+
(
1299+
DataFlow::localFlow(any(SensitiveDataSource src), name)
1300+
or
1301+
name = sensitiveLookupStringConst(_)
1302+
)
1303+
)
1304+
}
1305+
12931306
/**
12941307
* Holds if the `Secure` flag of the cookie is known to have a value of `b`.
12951308
*/

‎python/ql/lib/semmle/python/dataflow/new/SensitiveDataSources.qll‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,3 +334,5 @@ private module SensitiveDataModeling {
334334
}
335335

336336
predicate sensitiveDataExtraStepForCalls = SensitiveDataModeling::extraStepForCalls/2;
337+
338+
predicate sensitiveLookupStringConst = SensitiveDataModeling::sensitiveLookupStringConst/1;

‎python/ql/src/Security/CWE-1004/NonHttpOnlyCookie.qhelp‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Cookies without the <code>HttpOnly</code> flag set are accessible to JavaScript running in the same origin.
8+
In case of a Cross-Site Scripting (XSS) vulnerability, the cookie can be stolen by a malicious script.
9+
If a sensitive cookie does not need to be accessed directly by client-side JS, the <code>HttpOnly</code> flag should be set.</p>
10+
</overview>
11+
12+
<recommendation>
13+
<p>Set <code>httponly</code> to <code>True</code>, or add <code>; HttpOnly;</code> to the cookie's raw header value, to ensure that the cookie is not accessible via JavaScript.</p>
14+
</recommendation>
15+
16+
<example>
17+
<p>In the following examples, the cases marked GOOD show secure cookie attributes being set; whereas in the case marked BAD they are not set.</p>
18+
<sample src="examples/InsecureCookie.py" />
19+
</example>
20+
21+
<references>
22+
<li>PortSwigger: <a href="https://portswigger.net/kb/issues/00500600_cookie-without-httponly-flag-set">Cookie without HttpOnly flag set</a></li>
23+
<li>MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie">Set-Cookie</a>.</li>
24+
</references>
25+
26+
</qhelp>

‎python/ql/src/Security/CWE-1004/NonHttpOnlyCookie.ql‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Sensitive cookie missing `HttpOnly` attribute.
3+
* @description Cookies without the `HttpOnly` attribute set can be accessed by JS scripts, making them more vulnerable to XSS attacks.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @security-severity 5.0
7+
* @precision high
8+
* @id py/client-exposed-cookie
9+
* @tags security
10+
* external/cwe/cwe-1004
11+
*/
12+
13+
import python
14+
import semmle.python.dataflow.new.DataFlow
15+
import semmle.python.Concepts
16+
17+
from Http::Server::CookieWrite cookie
18+
where
19+
cookie.hasHttpOnlyFlag(false) and
20+
cookie.isSensitive()
21+
select cookie, "Sensitive server cookie is set without HttpOnly flag."

‎python/ql/src/Security/CWE-1004/examples/InsecureCookie.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
from flask import Flask, request, make_response, Response
2+
3+
4+
@app.route("/good1")
5+
def good1():
6+
resp = make_response()
7+
resp.set_cookie("sessionid", value="value", secure=True, httponly=True, samesite='Strict') # GOOD: Attributes are securely set
8+
return resp
9+
10+
11+
@app.route("/good2")
12+
def good2():
13+
resp = make_response()
14+
resp.headers['Set-Cookie'] = "sessionid=value; Secure; HttpOnly; SameSite=Strict" # GOOD: Attributes are securely set
15+
return resp
16+
17+
@app.route("/bad1")
18+
def bad1():
19+
resp = make_response()
20+
resp.set_cookie("sessionid", value="value", samesite='None') # BAD: the SameSite attribute is set to 'None' and the 'Secure' and 'HttpOnly' attributes are set to False by default.
21+
return resp

‎python/ql/src/Security/CWE-1275/SameSiteNoneCookie.qhelp‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Cookies with the <code>SameSite</code> attribute set to <code>'None'</code> will be sent with cross-origin requests.
8+
This can sometimes allow for Cross-Site Request Forgery (CSRF) attacks, in which a third-party site could perform actions on behalf of a user, if the cookie is used for authentication.</p>
9+
</overview>
10+
11+
<recommendation>
12+
<p>Set the <code>samesite</code> to <code>Lax</code> or <code>Strict</code>, or add <code>; SameSite=Lax;</code>, or
13+
<code>; SameSite=Strict;</code> to the cookie's raw header value. The default value in most cases is <code>Lax</code>.</p>
14+
</recommendation>
15+
16+
<example>
17+
<p>In the following examples, the cases marked GOOD show secure cookie attributes being set; whereas in the case marked BAD they are not set.</p>
18+
<sample src="examples/InsecureCookie.py" />
19+
</example>
20+
21+
<references>
22+
<li>MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie">Set-Cookie</a>.</li>
23+
<li>OWASP: <a href="https://owasp.org/www-community/SameSite">SameSite</a>.</li>
24+
</references>
25+
26+
</qhelp>

‎python/ql/src/Security/CWE-1275/SameSiteNoneCookie.ql‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Sensitive cookie with `SameSite` attribute set to `None`.
3+
* @description Cookies with `SameSite` set to `None` can allow for Cross-Site Request Forgery (CSRF) attacks.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @security-severity 4.0
7+
* @precision high
8+
* @id py/samesite-none-cookie
9+
* @tags security
10+
* external/cwe/cwe-1275
11+
*/
12+
13+
import python
14+
import semmle.python.dataflow.new.DataFlow
15+
import semmle.python.Concepts
16+
17+
from Http::Server::CookieWrite cookie
18+
where
19+
cookie.hasSameSiteAttribute(any(Http::Server::CookieWrite::SameSiteNone v)) and
20+
cookie.isSensitive()
21+
select cookie, "Sensitive cookie with SameSite set to 'None'."

0 commit comments

Comments
 (0)