#14808 closed Bug (fixed)
i18n is not safe.
Reported by: | Stephen Kelly | Owned by: | Tobias Kunze |
---|---|---|---|
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 (last modified by )
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.
Change History (14)
comment:1 by , 14 years ago
Description: | modified (diff) |
---|
comment:2 by , 14 years ago
Cc: | added |
---|
I'm not sure this can be fixed at all. What about a translatable string which contains markup? Example: "<b>Today</b>", translated in the French po file by "<b>Aujourd'hui</b>".
If there is anything that can cause security problems in the translated strings, shouldn't they be fixed in the po file in the first place?
comment:3 by , 14 years ago
See the discussion in #10449, I think it is closely related to this ticket.
comment:4 by , 14 years ago
milestone: | 1.3 |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:5 by , 14 years ago
claudep:
I'm not sure this can be fixed at all. What about a translatable string which contains markup? Example: "<b>Today</b>", translated in the French po file by "<b>Aujourd'hui</b>".
That should be translated as "<b>Aujourd'hui</b>" because it it should be safe html output. Safe html in, safe html out. If the input string contains html, the translated output should be safe. That requires translators to know about such things of course though, or for template authors to use context telling the translators that the output should be safe html.
I don't think this issue can be solved within fixes to .po files though.
comment:6 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:7 by , 14 years ago
Cc: | added |
---|
comment:10 by , 10 years ago
Component: | Template system → Documentation |
---|
I believe the only way forwards is to explain the current behavior and add warnings in the documentation.
comment:11 by , 6 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
Added formatting.