Enhance I18nBehavior with parseHTMLSubset functionality |
||||
Issue description
Problem:
There's a number of translated strings with embedded links in them, and they are usually injected into HTML with:
foo.innerHTML = this.i18n('bar')
or
<foo inner-h-t-m-l=[[i18n('bar')]]>
These strings often also contain run time data such as usernames, and thus present an XSS vector.
Proposed solution:
Enhance I18nBehavior with a method that calls parseHTMLSubset to filter out dangerous HTML.
New usage becomes something like:
foo.innerHTML = this.i18nSantizeHtml('bar')
or
<foo inner-h-t-m-l=[[i18nSanitizeHtml('bar')]]>
,
Apr 20 2016
Please consider making the shorter this.i18n()
sanitize and making the longer named function
skip it. Maybe this.i18nUnsafe() or
this.i18nNoSanitizeHtml().
The goal for the above is to make the shorter
(lazy to type) function name the safe version
and make be more clear and verbose when an
alternative is warranted.
We may not even need this.i18nNoSanitizeHtml()
now that we have $i18n{} for static strings;
meaning that the remaining strings that would
need dynamic handling in JS are also likely to
need sanitization.
,
Apr 20 2016
Yes I agree with Dave's thinking from the security perspective.
The only strings that absolutely require using this.i18n() instead of $i18n are the ones with string interpolation of runtime data - which should be sanitized.
All the other strings could use $18n{} and be hidden and shown via CSS - which is safer than futzing with the DOM text content at runtime.
,
Apr 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/744f96d502ab03be5baf879e563cfc01cb336c4a commit 744f96d502ab03be5baf879e563cfc01cb336c4a Author: dbeam <dbeam@chromium.org> Date: Thu Apr 21 00:25:05 2016 WebUI: allow target="_blank" by default in parseHtmlSubset(). This code was originally written for the New Tab Page[1], which should always be clobbered by a link click. The NTP no longer shows promos (which is why it used parseHtmlSubset()), and other WebUI pages like settings use target="_blank", so allow target="_blank" globally. R=tommycli@chromium.org BUG= 605226 [1] https://codereview.chromium.org/1759007, https://crbug.com/29084 Review URL: https://codereview.chromium.org/1907653002 Cr-Commit-Position: refs/heads/master@{#388629} [modify] https://crrev.com/744f96d502ab03be5baf879e563cfc01cb336c4a/chrome/test/data/webui/parse_html_subset_test.html [modify] https://crrev.com/744f96d502ab03be5baf879e563cfc01cb336c4a/ui/webui/resources/js/parse_html_subset.js
,
May 2 2016
,
May 4 2016
,
May 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/20bb518435ea84cea2584891f4bf2a4180814bf9 commit 20bb518435ea84cea2584891f4bf2a4180814bf9 Author: mahmadi <mahmadi@chromium.org> Date: Fri May 06 14:44:03 2016 Refactors parse_html_subset() into i18n_behavior. R=dbeam@chromium.org BUG= 605226 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/1920253002 Cr-Commit-Position: refs/heads/master@{#392057} [modify] https://crrev.com/20bb518435ea84cea2584891f4bf2a4180814bf9/chrome/browser/resources/md_user_manager/compiled_resources2.gyp [modify] https://crrev.com/20bb518435ea84cea2584891f4bf2a4180814bf9/chrome/browser/resources/md_user_manager/create_profile.html [modify] https://crrev.com/20bb518435ea84cea2584891f4bf2a4180814bf9/chrome/browser/resources/md_user_manager/create_profile.js [modify] https://crrev.com/20bb518435ea84cea2584891f4bf2a4180814bf9/chrome/browser/resources/md_user_manager/supervised_user_create_confirm.html [modify] https://crrev.com/20bb518435ea84cea2584891f4bf2a4180814bf9/chrome/browser/resources/md_user_manager/supervised_user_create_confirm.js [modify] https://crrev.com/20bb518435ea84cea2584891f4bf2a4180814bf9/ui/webui/resources/html/i18n_behavior.html [modify] https://crrev.com/20bb518435ea84cea2584891f4bf2a4180814bf9/ui/webui/resources/js/compiled_resources2.gyp [modify] https://crrev.com/20bb518435ea84cea2584891f4bf2a4180814bf9/ui/webui/resources/js/i18n_behavior.js [modify] https://crrev.com/20bb518435ea84cea2584891f4bf2a4180814bf9/ui/webui/resources/js/parse_html_subset.js
,
May 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d70d9aaff0a51af4e69674be5ad9aaac07230b4a commit d70d9aaff0a51af4e69674be5ad9aaac07230b4a Author: dbeam <dbeam@chromium.org> Date: Fri May 06 22:51:41 2016 I18nBehavior: make i18nRaw private as raw_, add tests R=mahmadi@chromium.org BUG= 605226 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/1952253004 Cr-Commit-Position: refs/heads/master@{#392192} [add] https://crrev.com/d70d9aaff0a51af4e69674be5ad9aaac07230b4a/chrome/test/data/webui/i18n_behavior_test.html [modify] https://crrev.com/d70d9aaff0a51af4e69674be5ad9aaac07230b4a/chrome/test/data/webui/webui_resource_browsertest.cc [modify] https://crrev.com/d70d9aaff0a51af4e69674be5ad9aaac07230b4a/ui/webui/resources/js/i18n_behavior.js
,
Jun 8 2016
|
||||
►
Sign in to add a comment |
||||
Comment 1 by tommycli@chromium.org
, Apr 20 2016Owner: mahmadi@chromium.org