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

Merge pull request #14112 from erik-krogh/pyAllowedHosts

Browse Source 
Py: add sanitizer guard for `url_has_allowed_host_and_scheme`
This commit is contained in:
Erik Krogh Kristensen
2023-09-13 12:59:38 +02:00
committed by GitHub
parent 7400b4741e 7292730391
commit cd5973764b
4 changed files with 57 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
python/ql/lib/semmle/python/frameworks/Django.qll
View File

@@ -15,6 +15,7 @@ private import semmle.python.regex
private import semmle.python.frameworks.internal.PoorMansFunctionResolution
private import semmle.python.frameworks.internal.SelfRefMixin
private import semmle.python.frameworks.internal.InstanceTaintStepsHelper
private import semmle.python.security.dataflow.UrlRedirectCustomizations
/**
* INTERNAL: Do not use.
@@ -2788,4 +2789,31 @@ module PrivateDjango {
override predicate csrfEnabled() { decoratorName in ["csrf_protect", "requires_csrf_token"] }
}
private predicate djangoUrlHasAllowedHostAndScheme(
DataFlow::GuardNode g, ControlFlowNode node, boolean branch
) {
exists(API::CallNode call |
call =
API::moduleImport("django")
.getMember("utils")
.getMember("http")
.getMember("url_has_allowed_host_and_scheme")
.getACall() and
g = call.asCfgNode() and
node = call.getParameter(0, "url").asSink().asCfgNode() and
branch = true
)
}
/**
* A call to `django.utils.http.url_has_allowed_host_and_scheme`, considered as a sanitizer-guard for URL redirection.
*
* See https://docs.djangoproject.com/en/4.2/_modules/django/utils/http/
*/
private class DjangoAllowedUrl extends UrlRedirect::Sanitizer {
DjangoAllowedUrl() {
this = DataFlow::BarrierGuard<djangoUrlHasAllowedHostAndScheme/3>::getABarrierNode()
}
}
}

4
python/ql/src/change-notes/2023-09-13-django-url-allowed-host.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Improved _URL redirection from remote source_ (`py/url-redirection`) query to not alert when URL has been checked with `django.utils.http. url_has_allowed_host_and_scheme`.

11
python/ql/test/query-tests/Security/CWE-601-UrlRedirect/UrlRedirect.expected
View File

@@ -8,6 +8,7 @@ edges
| test.py:1:26:1:32 | GSSA Variable request | test.py:67:17:67:23 | ControlFlowNode for request |
| test.py:1:26:1:32 | GSSA Variable request | test.py:74:17:74:23 | ControlFlowNode for request |
| test.py:1:26:1:32 | GSSA Variable request | test.py:81:17:81:23 | ControlFlowNode for request |
| test.py:1:26:1:32 | GSSA Variable request | test.py:90:17:90:23 | ControlFlowNode for request |
| test.py:7:5:7:10 | SSA variable target | test.py:8:21:8:26 | ControlFlowNode for target |
| test.py:7:14:7:20 | ControlFlowNode for request | test.py:7:14:7:25 | ControlFlowNode for Attribute |
| test.py:7:14:7:25 | ControlFlowNode for Attribute | test.py:7:14:7:43 | ControlFlowNode for Attribute() |
@@ -47,6 +48,10 @@ edges
| test.py:81:17:81:28 | ControlFlowNode for Attribute | test.py:81:17:81:46 | ControlFlowNode for Attribute() |
| test.py:81:17:81:46 | ControlFlowNode for Attribute() | test.py:81:5:81:13 | SSA variable untrusted |
| test.py:82:5:82:10 | SSA variable unsafe | test.py:83:21:83:26 | ControlFlowNode for unsafe |
| test.py:90:5:90:13 | SSA variable untrusted | test.py:93:18:93:26 | ControlFlowNode for untrusted |
| test.py:90:17:90:23 | ControlFlowNode for request | test.py:90:17:90:28 | ControlFlowNode for Attribute |
| test.py:90:17:90:28 | ControlFlowNode for Attribute | test.py:90:17:90:46 | ControlFlowNode for Attribute() |
| test.py:90:17:90:46 | ControlFlowNode for Attribute() | test.py:90:5:90:13 | SSA variable untrusted |
nodes
| test.py:1:26:1:32 | ControlFlowNode for ImportMember | semmle.label | ControlFlowNode for ImportMember |
| test.py:1:26:1:32 | GSSA Variable request | semmle.label | GSSA Variable request |
@@ -97,6 +102,11 @@ nodes
| test.py:81:17:81:46 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| test.py:82:5:82:10 | SSA variable unsafe | semmle.label | SSA variable unsafe |
| test.py:83:21:83:26 | ControlFlowNode for unsafe | semmle.label | ControlFlowNode for unsafe |
| test.py:90:5:90:13 | SSA variable untrusted | semmle.label | SSA variable untrusted |
| test.py:90:17:90:23 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
| test.py:90:17:90:28 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
| test.py:90:17:90:46 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| test.py:93:18:93:26 | ControlFlowNode for untrusted | semmle.label | ControlFlowNode for untrusted |
subpaths
#select
| test.py:8:21:8:26 | ControlFlowNode for target | test.py:1:26:1:32 | ControlFlowNode for ImportMember | test.py:8:21:8:26 | ControlFlowNode for target | Untrusted URL redirection depends on a $@. | test.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
@@ -107,3 +117,4 @@ subpaths
| test.py:69:21:69:26 | ControlFlowNode for unsafe | test.py:1:26:1:32 | ControlFlowNode for ImportMember | test.py:69:21:69:26 | ControlFlowNode for unsafe | Untrusted URL redirection depends on a $@. | test.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
| test.py:76:21:76:26 | ControlFlowNode for unsafe | test.py:1:26:1:32 | ControlFlowNode for ImportMember | test.py:76:21:76:26 | ControlFlowNode for unsafe | Untrusted URL redirection depends on a $@. | test.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
| test.py:83:21:83:26 | ControlFlowNode for unsafe | test.py:1:26:1:32 | ControlFlowNode for ImportMember | test.py:83:21:83:26 | ControlFlowNode for unsafe | Untrusted URL redirection depends on a $@. | test.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |
| test.py:93:18:93:26 | ControlFlowNode for untrusted | test.py:1:26:1:32 | ControlFlowNode for ImportMember | test.py:93:18:93:26 | ControlFlowNode for untrusted | Untrusted URL redirection depends on a $@. | test.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |

14
python/ql/test/query-tests/Security/CWE-601-UrlRedirect/test.py
View File

@@ -81,3 +81,17 @@ def not_ok4():
untrusted = request.args.get('target', '')
unsafe = "%s?login=success" % untrusted
return redirect(unsafe, code=302)
from django.utils.http import url_has_allowed_host_and_scheme
import math
@app.route('/ok6')
def ok6():
untrusted = request.args.get('target', '')
# random chance.
if math.random() > 0.5:
redirect(untrusted, code=302) # NOT OK
if url_has_allowed_host_and_scheme(untrusted, allowed_hosts=None):
return redirect(untrusted, code=302) # OK
return redirect("https://example.com", code=302) # OK