Opened 14 years ago
Last modified 6 years ago
#14808 closed
i18n is not safe. — at Initial Version
Reported by: | Stephen Kelly | Owned by: | nobody |
---|---|---|---|
Component: | Documentation | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Claude Paroz, d1b | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
This was reported to security@, but was considered a regular bug, not something requiring a security release because it is not a practical attack vector.
If an attacker knows that the string "Today" (several others from the
fr-fr catalog) appears in a Template, they can switch to the french
locale to get an unescaped '.
In [1]: from django.template import Template, Context, mark_safe
In [2]: c = Context()
In [3]: t = Template('{% load i18n %}{% trans "Today" %}')
In [4]: t.render(c)
Out[4]: u"Aujourd'hui"
Note: the ' should be escaped to '
In [5]:
In [6]: c = Context({"today_var": mark_safe("Today")})
In [7]: t = Template('{% load i18n %}{% trans today_var %}')
In [8]: t.render(c)
Out[8]: u"Aujourd'hui"
I recommend this patch:
Index: django/utils/translation/trans_real.py
===================================================================
--- django/utils/translation/trans_real.py (revision 14499)
+++ django/utils/translation/trans_real.py (working copy)
@@ -9,7 +9,6 @@
from cStringIO import StringIO
from django.utils.importlib import import_module
-from django.utils.safestring import mark_safe, SafeData
from django.utils.thread_support import currentThread
# Translations are cached in a dictionary for every language+app tuple.
@@ -272,8 +271,7 @@
from django.conf import settings
_default = translation(settings.LANGUAGE_CODE)
result = getattr(_default, translation_function)(eol_message)
- if isinstance(message, SafeData):
- return mark_safe(result)
+ # The result of a i18n call is not safe.
return result
Result:
In [1]: from django.template import Template, Context, mark_safe
In [2]: c = Context()
In [3]: t = Template('{% load i18n %}{% trans "Today" %}')
In [4]: t.render(c)
Out[4]: u'Aujourd'hui'
In [5]:
In [6]: c = Context({"today_var": mark_safe("Today")})
In [7]: t = Template('{% load i18n %}{% trans today_var %}')
In [8]: t.render(c)
Out[8]: u'Aujourd'hui'
Additionally, the documentation should be updated to say that strings
marked for translation in templates should not be marked as safe:
{% trans "Today & Tomorrow"|safe|upper %} Looks safe, but gets
translated to French with a '.
This is only actually safe if i18n is not used, which is why
trans_null still has the ifinstance(SafeData) stuff.
With the patch above and the template {% trans today_and_tomo_var %}
If USE_I18N is off:
context.insert("today_and_tomo_var", mark_safe("Today & Tomorrow"))
Ok renders "Today & Tomorrow"
If USE_I18N is on:
context.insert("today_and_tomo_var", mark_safe("Today & Tomorrow"))
Not Ok. Renders "Today & Tomorrow" because the safe-ness is removed.
To solve that django users would need to be advised *not* to
pre-mark_safe translatable strings before putting them in the Context
*if* USE_I18N is true.
If USE_I18N is on:
context.insert("today_and_tomo_var", "Today & Tomorrow")
Ok. Renders "Today & Tomorrow" because
_render_value_in_context escapes the non-safe data.
The many ways strings could be dealt with make this a tricky issue to solve.