Skip to content

Commit fd7bd19

Browse files
szuendmibrunin
authored andcommitted
[Backport] Security bug 420885124
Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/6606957: Don't create remote objects for non-shared 'Error' objects Since the page can install accessors on various 'Error' prototypes, its possible to leak information from cross-origin scripts (via the inspector remote object). This CL fixes this particular instance by not creating remote objects for Errors where the 'SharedCrossOrigin' bit is not set in the originating script. Note that to do this, we have to find the right script and get it's origin info. We can't go via the debugger agent as it has not necessarily been enabled and so the script map in the debugger agent could be empty. Drive-by: There is one more oddity we don't fully understand. The scriptId passed via V8Inspector is set to 0 if it matches the top-most frame. This CL reverts this logic in the inspector, but this might break some expectation in DevTools. [email protected] Fixed: 420885124 Change-Id: I3c1a7524349cdbadd6768f8c6bf5119d4b59369a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6606957 Reviewed-by: Benedikt Meurer <[email protected]> Commit-Queue: Simon Zünd <[email protected]> Cr-Commit-Position: refs/heads/main@{#100606} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/659329 Reviewed-by: Moss Heim <[email protected]>
1 parent 4c0d2d7 commit fd7bd19

File tree

3 files changed

+56
-4
lines changed

3 files changed

+56
-4
lines changed

chromium/v8/src/debug/debug-interface.cc

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -973,6 +973,40 @@ void GetLoadedScripts(Isolate* v8_isolate,
973973
}
974974
}
975975

976+
std::optional<v8::ScriptOrigin> GetScriptOrigin(Isolate* v8_isolate,
977+
int script_id) {
978+
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
979+
ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
980+
{
981+
i::DisallowGarbageCollection no_gc;
982+
i::Script::Iterator iterator(isolate);
983+
for (i::Tagged<i::Script> script = iterator.Next(); !script.is_null();
984+
script = iterator.Next()) {
985+
if (script->id() == script_id) {
986+
i::DirectHandle<i::Object> scriptName(script->GetNameOrSourceURL(),
987+
isolate);
988+
i::DirectHandle<i::Object> source_map_url(script->source_mapping_url(),
989+
isolate);
990+
i::DirectHandle<i::Object> host_defined_options(
991+
script->host_defined_options(), isolate);
992+
ScriptOriginOptions options(script->origin_options());
993+
bool is_wasm = false;
994+
#if V8_ENABLE_WEBASSEMBLY
995+
is_wasm = script->type() == i::Script::Type::kWasm;
996+
#endif // V8_ENABLE_WEBASSEMBLY
997+
v8::ScriptOrigin origin(
998+
Utils::ToLocal(scriptName), script->line_offset(),
999+
script->column_offset(), options.IsSharedCrossOrigin(),
1000+
script->id(), Utils::ToLocal(source_map_url), options.IsOpaque(),
1001+
is_wasm, options.IsModule(), Utils::ToLocal(host_defined_options));
1002+
return origin;
1003+
}
1004+
}
1005+
}
1006+
1007+
return std::nullopt;
1008+
}
1009+
9761010
MaybeLocal<UnboundScript> CompileInspectorScript(Isolate* v8_isolate,
9771011
Local<String> source) {
9781012
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);

chromium/v8/src/debug/debug-interface.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,9 @@ void Disassemble(base::Vector<const uint8_t> wire_bytes,
285285
V8_EXPORT_PRIVATE void GetLoadedScripts(
286286
Isolate* isolate, std::vector<v8::Global<Script>>& scripts);
287287

288+
V8_EXPORT_PRIVATE std::optional<v8::ScriptOrigin> GetScriptOrigin(
289+
Isolate* v8_isolate, int script_id);
290+
288291
MaybeLocal<UnboundScript> CompileInspectorScript(Isolate* isolate,
289292
Local<String> source);
290293

chromium/v8/src/inspector/v8-console-message.cc

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,9 @@ void V8ConsoleMessage::setLocation(const String16& url, unsigned lineNumber,
220220
m_columnNumber = columnNumber;
221221
m_stackTrace = std::move(stackTrace);
222222
m_scriptId = scriptId;
223+
if (!m_scriptId && m_stackTrace && !m_stackTrace->frames().empty()) {
224+
m_scriptId = m_stackTrace->frames()[0].scriptId;
225+
}
223226
}
224227

225228
void V8ConsoleMessage::reportToFrontend(
@@ -317,13 +320,23 @@ void V8ConsoleMessage::reportToFrontend(protocol::Runtime::Frontend* frontend,
317320
v8::debug::PostponeInterruptsScope no_interrupts(inspector->isolate());
318321

319322
if (m_origin == V8MessageOrigin::kException) {
320-
std::unique_ptr<protocol::Runtime::RemoteObject> exception =
321-
wrapException(session, generatePreview);
322323
if (!inspector->hasConsoleMessageStorage(contextGroupId)) return;
324+
v8::HandleScope scope(inspector->isolate());
325+
auto maybeScriptOrigin =
326+
v8::debug::GetScriptOrigin(inspector->isolate(), m_scriptId);
327+
const bool isSharedCrossOrigin =
328+
maybeScriptOrigin ? maybeScriptOrigin->Options().IsSharedCrossOrigin()
329+
: false;
330+
std::unique_ptr<protocol::Runtime::RemoteObject> exception =
331+
isSharedCrossOrigin ? wrapException(session, generatePreview) : nullptr;
332+
const bool includeException = isSharedCrossOrigin && exception;
323333
std::unique_ptr<protocol::Runtime::ExceptionDetails> exceptionDetails =
324334
protocol::Runtime::ExceptionDetails::create()
325335
.setExceptionId(m_exceptionId)
326-
.setText(exception ? m_message : m_detailedMessage)
336+
.setText(includeException
337+
? m_message
338+
: (m_detailedMessage.length() ? m_detailedMessage
339+
: m_message))
327340
.setLineNumber(m_lineNumber ? m_lineNumber - 1 : 0)
328341
.setColumnNumber(m_columnNumber ? m_columnNumber - 1 : 0)
329342
.build();
@@ -335,7 +348,9 @@ void V8ConsoleMessage::reportToFrontend(protocol::Runtime::Frontend* frontend,
335348
m_stackTrace->buildInspectorObjectImpl(inspector->debugger()));
336349
}
337350
if (m_contextId) exceptionDetails->setExecutionContextId(m_contextId);
338-
if (exception) exceptionDetails->setException(std::move(exception));
351+
if (includeException) {
352+
exceptionDetails->setException(std::move(exception));
353+
}
339354
std::unique_ptr<protocol::DictionaryValue> data =
340355
getAssociatedExceptionData(inspector, session);
341356
if (data) exceptionDetails->setExceptionMetaData(std::move(data));

0 commit comments

Comments
 (0)