Commit 9e6f6667 authored by Jakub Vrana's avatar Jakub Vrana Committed by Commit Bot

Add option to escape < in chrome.i18n.getMessage

Translations are not always under the application control and they can
contain special characters. If they are used in HTML context then this
can result in XSS.

This change addresses it by escaping < in chrome.i18n.getMessage before
substituting the placeholders (which often contain trusted HTML) if the
new {escape_lt: true} option is set.

This will be used by Closure Templates which generate
goog.getMsg('', {}, {html: true}) which will be translated by Closure
Compiler to chrome.i18n.getMessage('', {}, {escape_lt: true}) for
Chrome extensions.

Bug: 989413
Change-Id: I5c56af375dc443a0f6fc7ebddc038fb4a074db3c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1728572
Commit-Queue: Jakub Vrana <jakubvrana@google.com>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Auto-Submit: Jakub Vrana <jakubvrana@google.com>
Cr-Commit-Position: refs/heads/master@{#694683}
parent fd7196ae
......@@ -44,6 +44,18 @@
"name": "substitutions",
"optional": true,
"description": "Up to 9 substitution strings, if the message requires any."
},
{
"type": "object",
"name": "options",
"optional": true,
"properties": {
"escapeLt": {
"type": "boolean",
"optional": true,
"description": "Escape <code>&lt;</code> in translation to <code>&amp;lt;</code>. This applies only to the message itself, not to the placeholders. Developers might want to use this if the translation is used in an HTML context. Closure Templates used with Closure Compiler generate this automatically."
}
}
}
],
"returns": {
......
......@@ -38,7 +38,8 @@ TEST_F(I18nHooksDelegateTest, TestI18nGetMessage) {
L10nMessagesMap messages = {
{"simple", "simple message"},
{"one_placeholder", "placeholder $1 end"},
{"multi_placeholders", "placeholder $1 and $2 end"}};
{"multi_placeholders", "placeholder $1 and $2 end"},
{"special_characters", "< Hello $1 World &gt;"}};
GetExtensionToL10nMessagesMap()->emplace(extension->id(), messages);
auto run_get_message = [context](const char* args) {
......@@ -60,6 +61,14 @@ TEST_F(I18nHooksDelegateTest, TestI18nGetMessage) {
run_get_message("'one_placeholder', ['foo']"));
EXPECT_EQ(R"("placeholder foo and bar end")",
run_get_message("'multi_placeholders', ['foo', 'bar']"));
EXPECT_EQ(R"("\u003C Hello \u003Cbr> World &gt;")",
run_get_message("'special_characters', ['<br>'], {}"));
EXPECT_EQ(
R"("\u003C Hello \u003Cbr> World &gt;")",
run_get_message("'special_characters', ['<br>'], {escapeLt: false}"));
EXPECT_EQ(
R"("&lt; Hello \u003Cbr> World &gt;")",
run_get_message("'special_characters', ['<br>'], {escapeLt: true}"));
// We place the somewhat-arbitrary (but documented) limit of 9 substitutions
// on the call.
......
......@@ -9,6 +9,7 @@
#include "base/logging.h"
#include "base/metrics/histogram_macros.h"
#include "base/stl_util.h"
#include "base/strings/string_util.h"
#include "content/public/renderer/render_frame.h"
#include "content/public/renderer/render_thread.h"
#include "extensions/common/extension.h"
......@@ -141,6 +142,7 @@ void InitDetectedLanguages(
v8::Local<v8::Value> GetI18nMessage(const std::string& message_name,
const std::string& extension_id,
v8::Local<v8::Value> v8_substitutions,
v8::Local<v8::Value> v8_options,
content::RenderFrame* render_frame,
v8::Local<v8::Context> context) {
v8::Isolate* isolate = context->GetIsolate();
......@@ -201,6 +203,17 @@ v8::Local<v8::Value> GetI18nMessage(const std::string& message_name,
// for substitutions, but the type is documented as 'any'. We should either
// enforce type more heavily, or throw an error here.
if (v8_options->IsObject()) {
v8::Local<v8::Object> options = v8_options.As<v8::Object>();
v8::Local<v8::Value> key =
v8::String::NewFromUtf8(isolate, "escapeLt").ToLocalChecked();
v8::Local<v8::Value> html;
if (options->Get(context, key).ToLocal(&html) && html->IsBoolean() &&
html.As<v8::Boolean>()->Value()) {
base::ReplaceChars(message, "<", "&lt;", &message);
}
}
// NOTE: We call ReplaceStringPlaceholders even if |substitutions| is empty
// because we substitute $$ to be $ (in order to display a dollar sign in a
// message). See https://crbug.com/127243.
......@@ -287,7 +300,8 @@ RequestResult I18nHooksDelegate::HandleGetMessage(
v8::Local<v8::Value> message = GetI18nMessage(
gin::V8ToString(script_context->isolate(), parsed_arguments[0]),
script_context->extension()->id(), parsed_arguments[1],
script_context->GetRenderFrame(), script_context->v8_context());
parsed_arguments[2], script_context->GetRenderFrame(),
script_context->v8_context());
RequestResult result(RequestResult::HANDLED);
result.return_value = message;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment