Skip to content

Commit a833632

Browse files
authored
Merge pull request github#15176 from github/max-schaefer/py-url-redirection-qhelp
Python: Mention more sanitisation options in py/url-redirection qhelp.
2 parents 448439e + 66fe32a commit a833632

File tree

3 files changed

+39
-3
lines changed

3 files changed

+39
-3
lines changed

python/ql/src/Security/CWE-601/UrlRedirect.qhelp

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ To guard against untrusted URL redirection, it is advisable to avoid putting use
1616
directly into a redirect URL. Instead, maintain a list of authorized
1717
redirects on the server; then choose from that list based on the user input provided.
1818
</p>
19+
<p>
20+
If this is not possible, then the user input should be validated in some other way,
21+
for example, by verifying that the target URL does not include an explicit host name.
22+
</p>
1923
</recommendation>
2024

2125
<example>
@@ -27,11 +31,29 @@ without validating the input, which facilitates phishing attacks:
2731
<sample src="examples/redirect_bad.py"/>
2832

2933
<p>
30-
One way to remedy the problem is to validate the user input against a known fixed string
31-
before doing the redirection:
34+
If you know the set of valid redirect targets, you can maintain a list of them on the server
35+
and check that the user input is in that list:
3236
</p>
3337

3438
<sample src="examples/redirect_good.py"/>
39+
40+
<p>
41+
Often this is not possible, so an alternative is to check that the target URL does not
42+
specify an explicit host name. For example, the Django framework provides a
43+
function <code>url_has_allowed_host_and_scheme</code> that can be used to check that a
44+
URL is safe to redirect to, as shown in the following example:
45+
</p>
46+
47+
<sample src="examples/redirect_good2.py"/>
48+
49+
<p>
50+
Note that many browsers accept backslash characters (<code>\</code>) as equivalent to
51+
forward slash characters (<code>/</code>) in URLs, so it is important to account for this
52+
when validating the URL, for example by first replacing all backslashes with forward
53+
slashes. Django's <code>url_has_allowed_host_and_scheme</code> function
54+
does this automatically, but other libraries may not.
55+
</p>
56+
3557
</example>
3658

3759
<references>

python/ql/src/Security/CWE-601/examples/redirect_good.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,5 @@ def hello():
1010
if target == VALID_REDIRECT:
1111
return redirect(target, code=302)
1212
else:
13-
... # Error
13+
# ignore the target and redirect to the home page
14+
return redirect('/', code=302)
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
from django.http import HttpResponseRedirect
2+
from django.shortcuts import redirect
3+
from django.utils.http import url_has_allowed_host_and_scheme
4+
from django.views import View
5+
6+
class RedirectView(View):
7+
def get(self, request, *args, **kwargs):
8+
target = request.GET.get('target', '')
9+
if url_has_allowed_host_and_scheme(target, allowed_hosts=None):
10+
return HttpResponseRedirect(target)
11+
else:
12+
# ignore the target and redirect to the home page
13+
return redirect('/')

0 commit comments

Comments
 (0)