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>

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);
 }