Skip to content

Commit 4bb9a3c

Browse files
zyvtomchristie
authored andcommitted
Fix XSS caused by disabled autoescaping in the default DRF Browsable API view templates (#6330)
* Add test that verifies that HTML is correctly escaped in Browsable API views * Fix `urlize_quoted_links` tag to avoid double escaping in autoescape mode * Fix XSS in default DRF Browsable API template by re-enabling autoescape
1 parent e3bd4b9 commit 4bb9a3c

File tree

3 files changed

+26
-17
lines changed

3 files changed

+26
-17
lines changed

rest_framework/templates/rest_framework/base.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,10 +171,10 @@ <h1>{{ name }}</h1>
171171
</div>
172172

173173
<div class="response-info" aria-label="{% trans "response info" %}">
174-
<pre class="prettyprint"><span class="meta nocode"><b>HTTP {{ response.status_code }} {{ response.status_text }}</b>{% autoescape off %}{% for key, val in response_headers|items %}
174+
<pre class="prettyprint"><span class="meta nocode"><b>HTTP {{ response.status_code }} {{ response.status_text }}</b>{% for key, val in response_headers|items %}
175175
<b>{{ key }}:</b> <span class="lit">{{ val|break_long_headers|urlize_quoted_links }}</span>{% endfor %}
176176

177-
</span>{{ content|urlize_quoted_links }}</pre>{% endautoescape %}
177+
</span>{{ content|urlize_quoted_links }}</pre>
178178
</div>
179179
</div>
180180

rest_framework/templatetags/rest_framework.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,12 @@ def trim_url(x, limit=trim_url_limit):
336336
return limit is not None and (len(x) > limit and ('%s...' % x[:max(0, limit - 3)])) or x
337337

338338
safe_input = isinstance(text, SafeData)
339+
340+
# Unfortunately, Django built-in cannot be used here, because escaping
341+
# is to be performed on words, which have been forcibly coerced to text
342+
def conditional_escape(text):
343+
return escape(text) if autoescape and not safe_input else text
344+
339345
words = word_split_re.split(force_text(text))
340346
for i, word in enumerate(words):
341347
if '.' in word or '@' in word or ':' in word:
@@ -376,21 +382,15 @@ def trim_url(x, limit=trim_url_limit):
376382
# Make link.
377383
if url:
378384
trimmed = trim_url(middle)
379-
if autoescape and not safe_input:
380-
lead, trail = escape(lead), escape(trail)
381-
url, trimmed = escape(url), escape(trimmed)
385+
lead, trail = conditional_escape(lead), conditional_escape(trail)
386+
url, trimmed = conditional_escape(url), conditional_escape(trimmed)
382387
middle = '<a href="%s"%s>%s</a>' % (url, nofollow_attr, trimmed)
383-
words[i] = mark_safe('%s%s%s' % (lead, middle, trail))
388+
words[i] = '%s%s%s' % (lead, middle, trail)
384389
else:
385-
if safe_input:
386-
words[i] = mark_safe(word)
387-
elif autoescape:
388-
words[i] = escape(word)
389-
elif safe_input:
390-
words[i] = mark_safe(word)
391-
elif autoescape:
392-
words[i] = escape(word)
393-
return ''.join(words)
390+
words[i] = conditional_escape(word)
391+
else:
392+
words[i] = conditional_escape(word)
393+
return mark_safe(''.join(words))
394394

395395

396396
@register.filter

tests/test_templatetags.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,15 +305,24 @@ def test_json_with_url(self):
305305
'&quot;foo_set&quot;: [\n &quot;<a href="http://api/foos/1/">http://api/foos/1/</a>&quot;\n], '
306306
self._urlize_dict_check(data)
307307

308+
def test_template_render_with_autoescape(self):
309+
"""
310+
Test that HTML is correctly escaped in Browsable API views.
311+
"""
312+
template = Template("{% load rest_framework %}{{ content|urlize_quoted_links }}")
313+
rendered = template.render(Context({'content': '<script>alert()</script> http://example.com'}))
314+
assert rendered == '&lt;script&gt;alert()&lt;/script&gt;' \
315+
' <a href="http://example.com" rel="nofollow">http://example.com</a>'
316+
308317
def test_template_render_with_noautoescape(self):
309318
"""
310319
Test if the autoescape value is getting passed to urlize_quoted_links filter.
311320
"""
312321
template = Template("{% load rest_framework %}"
313322
"{% autoescape off %}{{ content|urlize_quoted_links }}"
314323
"{% endautoescape %}")
315-
rendered = template.render(Context({'content': '"http://example.com"'}))
316-
assert rendered == '"<a href="http://example.com" rel="nofollow">http://example.com</a>"'
324+
rendered = template.render(Context({'content': '<b> "http://example.com" </b>'}))
325+
assert rendered == '<b> "<a href="http://example.com" rel="nofollow">http://example.com</a>" </b>'
317326

318327

319328
@unittest.skipUnless(coreapi, 'coreapi is not installed')

0 commit comments

Comments
 (0)