Skip to content

Commit 8dad495

Browse files
committed
add sanitizer guard for url_has_allowed_host_and_scheme
1 parent d4bc6e4 commit 8dad495

File tree

3 files changed

+28
-4
lines changed

3 files changed

+28
-4
lines changed

python/ql/lib/semmle/python/security/dataflow/UrlRedirectCustomizations.qll

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,4 +70,31 @@ module UrlRedirect {
7070
* A comparison with a constant string, considered as a sanitizer-guard.
7171
*/
7272
class StringConstCompareAsSanitizerGuard extends Sanitizer, StringConstCompareBarrier { }
73+
74+
private import semmle.python.ApiGraphs
75+
76+
private predicate djangoUrlHasAllowedHostAndScheme(
77+
DataFlow::GuardNode g, ControlFlowNode node, boolean branch
78+
) {
79+
exists(API::CallNode call |
80+
call =
81+
API::moduleImport("django")
82+
.getMember("utils")
83+
.getMember("http")
84+
.getMember("url_has_allowed_host_and_scheme")
85+
.getACall() and
86+
g = call.asCfgNode() and
87+
node = call.getParameter(0, "url").asSink().asCfgNode() and
88+
branch = true
89+
)
90+
}
91+
92+
/**
93+
* A call to `django.utils.http.url_has_allowed_host_and_scheme`, considered as a sanitizer-guard.
94+
*/
95+
private class DjangoAllowedUrl extends Sanitizer {
96+
DjangoAllowedUrl() {
97+
this = DataFlow::BarrierGuard<djangoUrlHasAllowedHostAndScheme/3>::getABarrierNode()
98+
}
99+
}
73100
}

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ edges
4949
| test.py:81:17:81:46 | ControlFlowNode for Attribute() | test.py:81:5:81:13 | SSA variable untrusted |
5050
| test.py:82:5:82:10 | SSA variable unsafe | test.py:83:21:83:26 | ControlFlowNode for unsafe |
5151
| test.py:90:5:90:13 | SSA variable untrusted | test.py:93:18:93:26 | ControlFlowNode for untrusted |
52-
| test.py:90:5:90:13 | SSA variable untrusted | test.py:95:25:95:33 | ControlFlowNode for untrusted |
5352
| test.py:90:17:90:23 | ControlFlowNode for request | test.py:90:17:90:28 | ControlFlowNode for Attribute |
5453
| test.py:90:17:90:28 | ControlFlowNode for Attribute | test.py:90:17:90:46 | ControlFlowNode for Attribute() |
5554
| test.py:90:17:90:46 | ControlFlowNode for Attribute() | test.py:90:5:90:13 | SSA variable untrusted |
@@ -108,7 +107,6 @@ nodes
108107
| test.py:90:17:90:28 | ControlFlowNode for Attribute | semmle.label | ControlFlowNode for Attribute |
109108
| test.py:90:17:90:46 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
110109
| test.py:93:18:93:26 | ControlFlowNode for untrusted | semmle.label | ControlFlowNode for untrusted |
111-
| test.py:95:25:95:33 | ControlFlowNode for untrusted | semmle.label | ControlFlowNode for untrusted |
112110
subpaths
113111
#select
114112
| 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 |
@@ -120,4 +118,3 @@ subpaths
120118
| 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 |
121119
| 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 |
122120
| 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 |
123-
| test.py:95:25:95:33 | ControlFlowNode for untrusted | test.py:1:26:1:32 | ControlFlowNode for ImportMember | test.py:95:25:95:33 | ControlFlowNode for untrusted | Untrusted URL redirection depends on a $@. | test.py:1:26:1:32 | ControlFlowNode for ImportMember | user-provided value |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,6 @@ def ok6():
9292
if math.random() > 0.5:
9393
redirect(untrusted, code=302) # NOT OK
9494
if url_has_allowed_host_and_scheme(untrusted, allowed_hosts=None):
95-
return redirect(untrusted, code=302) # OK - but is flagged!
95+
return redirect(untrusted, code=302) # OK
9696

9797
return redirect("https://example.com", code=302) # OK

0 commit comments

Comments
 (0)