티스토리 수익 글 보기

티스토리 수익 글 보기

[1.8.x] Fixed #27912, CVE-2017-7233 — Fixed is_safe_url() with numer… · django/django@8339277 · GitHub
Skip to content

Commit 8339277

Browse files
committed
[1.8.x] Fixed #27912, CVE-2017-7233 — Fixed is_safe_url() with numeric URLs.
This is a security fix.
1 parent 4a6b945 commit 8339277

3 files changed

Lines changed: 81 additions & 2 deletions

File tree

django/utils/http.py

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,18 @@
1818
urlparse,
1919
)
2020

21+
if six.PY2:
22+
from urlparse import (
23+
ParseResult, SplitResult, _splitnetloc, _splitparams, scheme_chars,
24+
uses_params,
25+
)
26+
_coerce_args = None
27+
else:
28+
from urllib.parse import (
29+
ParseResult, SplitResult, _coerce_args, _splitnetloc, _splitparams,
30+
scheme_chars, uses_params,
31+
)
32+
2133
ETAG_MATCH = re.compile(r'(?:W/)?"((?:\\.|[^"])*)"')
2234

2335
MONTHS = 'jan feb mar apr may jun jul aug sep oct nov dec'.split()
@@ -287,12 +299,64 @@ def is_safe_url(url, host=None):
287299
return _is_safe_url(url, host) and _is_safe_url(url.replace('\\', '/'), host)
288300

289301

302+
# Copied from urllib.parse.urlparse() but uses fixed urlsplit() function.
303+
def _urlparse(url, scheme='', allow_fragments=True):
304+
"""Parse a URL into 6 components:
305+
<scheme>://<netloc>/<path>;<params>?<query>#<fragment>
306+
Return a 6-tuple: (scheme, netloc, path, params, query, fragment).
307+
Note that we don't break the components up in smaller bits
308+
(e.g. netloc is a single string) and we don't expand % escapes."""
309+
if _coerce_args:
310+
url, scheme, _coerce_result = _coerce_args(url, scheme)
311+
splitresult = _urlsplit(url, scheme, allow_fragments)
312+
scheme, netloc, url, query, fragment = splitresult
313+
if scheme in uses_params and ';' in url:
314+
url, params = _splitparams(url)
315+
else:
316+
params = ''
317+
result = ParseResult(scheme, netloc, url, params, query, fragment)
318+
return _coerce_result(result) if _coerce_args else result
319+
320+
321+
# Copied from urllib.parse.urlsplit() with
322+
# https://github.com/python/cpython/pull/661 applied.
323+
def _urlsplit(url, scheme='', allow_fragments=True):
324+
"""Parse a URL into 5 components:
325+
<scheme>://<netloc>/<path>?<query>#<fragment>
326+
Return a 5-tuple: (scheme, netloc, path, query, fragment).
327+
Note that we don't break the components up in smaller bits
328+
(e.g. netloc is a single string) and we don't expand % escapes."""
329+
if _coerce_args:
330+
url, scheme, _coerce_result = _coerce_args(url, scheme)
331+
allow_fragments = bool(allow_fragments)
332+
netloc = query = fragment = ''
333+
i = url.find(':')
334+
if i > 0:
335+
for c in url[:i]:
336+
if c not in scheme_chars:
337+
break
338+
else:
339+
scheme, url = url[:i].lower(), url[i + 1:]
340+
341+
if url[:2] == '//':
342+
netloc, url = _splitnetloc(url, 2)
343+
if (('[' in netloc and ']' not in netloc) or
344+
(']' in netloc and '[' not in netloc)):
345+
raise ValueError("Invalid IPv6 URL")
346+
if allow_fragments and '#' in url:
347+
url, fragment = url.split('#', 1)
348+
if '?' in url:
349+
url, query = url.split('?', 1)
350+
v = SplitResult(scheme, netloc, url, query, fragment)
351+
return _coerce_result(v) if _coerce_args else v
352+
353+
290354
def _is_safe_url(url, host):
291355
# Chrome considers any URL with more than two slashes to be absolute, but
292356
# urlparse is not so flexible. Treat any url with three slashes as unsafe.
293357
if url.startswith('///'):
294358
return False
295-
url_info = urlparse(url)
359+
url_info = _urlparse(url)
296360
# Forbid URLs like http:///example.com - with a scheme, but without a hostname.
297361
# In that URL, example.com is not the hostname but, a path component. However,
298362
# Chrome will still consider example.com to be the hostname, so we must not

docs/releases/1.8.18.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,18 @@ Django 1.8.18 release notes
66

77
Django 1.8.18 fixes two security issues in 1.8.17.
88

9+
CVE-2017-7233: Open redirect and possible XSS attack via user-supplied numeric redirect URLs
10+
============================================================================================
11+
12+
Django relies on user input in some cases (e.g.
13+
:func:`django.contrib.auth.views.login` and :doc:`i18n </topics/i18n/index>`)
14+
to redirect the user to an "on success" URL. The security check for these
15+
redirects (namely ``django.utils.http.is_safe_url()``) considered some numeric
16+
URLs (e.g. ``http:999999999``) "safe" when they shouldn't be.
17+
18+
Also, if a developer relies on ``is_safe_url()`` to provide safe redirect
19+
targets and puts such a URL into a link, they could suffer from an XSS attack.
20+
921
CVE-2017-7234: Open redirect vulnerability in ``django.views.static.serve()``
1022
=============================================================================
1123

tests/utils_tests/test_http.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ def test_is_safe_url(self):
123123
r'http://testserver\me:pass@example.com',
124124
r'http://testserver\@example.com',
125125
r'http:\\testserver\confirm\me@example.com',
126+
'http:999999999',
127+
'ftp:9999999999',
126128
'\n'):
127129
self.assertFalse(http.is_safe_url(bad_url, host='testserver'), "%s should be blocked" % bad_url)
128130
for good_url in ('/view/?param=http://example.com',
@@ -133,7 +135,8 @@ def test_is_safe_url(self):
133135
'HTTPS://testserver/',
134136
'//testserver/',
135137
'http://testserver/confirm?email=me@example.com',
136-
'/url%20with%20spaces/'):
138+
'/url%20with%20spaces/',
139+
'path/http:2222222222'):
137140
self.assertTrue(http.is_safe_url(good_url, host='testserver'), "%s should be allowed" % good_url)
138141

139142
if six.PY2:

0 commit comments

Comments
 (0)