diff options
author | Jüri Valdmann <juri.valdmann@qt.io> | 2018-04-27 15:48:17 +0200 |
---|---|---|
committer | Michal Klocek <michal.klocek@qt.io> | 2018-05-16 17:14:59 +0000 |
commit | 8476245d1a197d05f988ef87f17b7ccbbcbba878 (patch) | |
tree | 08a0d181a56fc4079403543a807600d584f8827a /src | |
parent | 580fdd43c23aa409880a64f7dc0ce04ec57a1bcd (diff) |
Replace invalid characters in WebChannel messages
Turns out JavaScript's JSON.stringify is not guaranteed to produce valid UTF-16
strings. It is possible in JavaScript to produce string objects which contain
invalid code units (unmatched surrogate pairs) and JSON.stringify will simply
copy this data to it's output. However, such a string cannot be losslessly
converted to UTF-8 and this leads to fun errors in WebChannelIPCTransport.
This patch
- Adds a test for the scenario above.
- Changes WebChannelIPCTransport to replace these invalid code units with the
Unicode replacement character U+FFFD.
- Changes WebChannelIPCTransportHost to validate the data it gets from the
renderer. Not validating the data defeats the whole point of Chromium's
fancy multi-process architecture: the renderer is not to be trusted.
- Changes WebChannelIPCTransport to throw JavaScript exceptions for various
errors (missing argument, wrong type, invalid JSON). Seems like the polite
thing to do.
Task-number: QTBUG-61969
Change-Id: I83275a0eaed77109dc458b80e27217108dde9f7b
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
Diffstat (limited to 'src')
-rw-r--r-- | src/core/renderer/web_channel_ipc_transport.cpp | 38 | ||||
-rw-r--r-- | src/core/renderer_host/web_channel_ipc_transport_host.cpp | 17 |
2 files changed, 37 insertions, 18 deletions
diff --git a/src/core/renderer/web_channel_ipc_transport.cpp b/src/core/renderer/web_channel_ipc_transport.cpp index bb544168f..ef00bcef3 100644 --- a/src/core/renderer/web_channel_ipc_transport.cpp +++ b/src/core/renderer/web_channel_ipc_transport.cpp @@ -64,7 +64,7 @@ public: static void Uninstall(blink::WebLocalFrame *frame, uint worldId); private: WebChannelTransport() {} - bool NativeQtSendMessage(gin::Arguments *args); + void NativeQtSendMessage(gin::Arguments *args); // gin::WrappableBase gin::ObjectTemplateBuilder GetObjectTemplateBuilder(v8::Isolate *isolate) override; @@ -118,37 +118,45 @@ void WebChannelTransport::Uninstall(blink::WebLocalFrame *frame, uint worldId) qtObject->Delete(gin::StringToV8(isolate, "webChannelTransport")); } -bool WebChannelTransport::NativeQtSendMessage(gin::Arguments *args) +void WebChannelTransport::NativeQtSendMessage(gin::Arguments *args) { blink::WebLocalFrame *frame = blink::WebLocalFrame::FrameForCurrentContext(); if (!frame || !frame->View()) - return false; + return; content::RenderFrame *renderFrame = content::RenderFrame::FromWebFrame(frame); if (!renderFrame) - return false; + return; + + v8::Local<v8::Value> jsonValue; + if (!args->GetNext(&jsonValue)) { + args->ThrowTypeError("Missing argument"); + return; + } - std::string message; - if (!args->GetNext(&message)) - return false; + if (!jsonValue->IsString()) { + args->ThrowTypeError("Expected string"); + return; + } + v8::Local<v8::String> jsonString = v8::Local<v8::String>::Cast(jsonValue); + + QByteArray json(jsonString->Utf8Length(), 0); + jsonString->WriteUtf8(json.data(), json.size(), + nullptr, + v8::String::REPLACE_INVALID_UTF8); - QByteArray valueData(message.data(), message.size()); QJsonParseError error; - QJsonDocument doc = QJsonDocument::fromJson(valueData, &error); + QJsonDocument doc = QJsonDocument::fromJson(json, &error); if (error.error != QJsonParseError::NoError) { - LOG(WARNING) << "Parsing error: " << qPrintable(error.errorString()); - return false; + args->ThrowTypeError("Invalid JSON"); + return; } int size = 0; const char *rawData = doc.rawData(&size); - if (size == 0) - return false; - renderFrame->Send(new WebChannelIPCTransportHost_SendMessage( renderFrame->GetRoutingID(), std::vector<char>(rawData, rawData + size))); - return true; } gin::ObjectTemplateBuilder WebChannelTransport::GetObjectTemplateBuilder(v8::Isolate *isolate) diff --git a/src/core/renderer_host/web_channel_ipc_transport_host.cpp b/src/core/renderer_host/web_channel_ipc_transport_host.cpp index 6b32093a6..d99dfde97 100644 --- a/src/core/renderer_host/web_channel_ipc_transport_host.cpp +++ b/src/core/renderer_host/web_channel_ipc_transport_host.cpp @@ -49,6 +49,8 @@ #include <QJsonObject> #include <QLoggingCategory> +#include <QtCore/private/qjson_p.h> + namespace QtWebEngineCore { Q_LOGGING_CATEGORY(log, "qt.webengine.webchanneltransport"); @@ -108,10 +110,19 @@ void WebChannelIPCTransportHost::setWorldId(content::RenderFrameHost *frame, bas void WebChannelIPCTransportHost::onWebChannelMessage(const std::vector<char> &message) { - Q_ASSERT(!message.empty()); - QJsonDocument doc = QJsonDocument::fromRawData(message.data(), message.size(), QJsonDocument::BypassValidation); - Q_ASSERT(doc.isObject()); content::RenderFrameHost *frame = web_contents()->GetMainFrame(); + + QJsonDocument doc; + // QJsonDocument::fromRawData does not check the length before it starts + // parsing the QJsonPrivate::Header and QJsonPrivate::Base structures. + if (message.size() >= sizeof(QJsonPrivate::Header) + sizeof(QJsonPrivate::Base)) + doc = QJsonDocument::fromRawData(message.data(), message.size()); + + if (!doc.isObject()) { + qCCritical(log).nospace() << "received invalid webchannel message from " << frame; + return; + } + qCDebug(log).nospace() << "received webchannel message from " << frame << ": " << doc; Q_EMIT messageReceived(doc.object(), this); } |