Skip to content

Commit d5c3918

Browse files
yaoxiachromiumChromium LUCI CQ
authored and
Chromium LUCI CQ
committed
[shared storage] During addModule(), switch to allow navigator but throw on navigator.locks
Currently, `navigator.locks` is the only available attribute on `navigator` (i.e., a SharedStorageWorkletNavigator), and we want to prevent Web Locks functionality during addModule(), because lock acquisition results (success, timing) could reveal data from other same-origin worklets, which could enable cross-site information leak via addModule() timing. This change avoids unintentionally breaking existing code that might legitimately read the navigator object (and expect no Exceptions thrown) during module loading for purposes unrelated to Web Locks. PR: WICG/shared-storage#227 Bug: 396148598 Change-Id: I67b217238912710ca89de319c6b7e1d9d2d1cbcf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6256882 Reviewed-by: Tsuyoshi Horo <[email protected]> Commit-Queue: Yao Xiao <[email protected]> Cr-Commit-Position: refs/heads/main@{#1419694}
1 parent 5438781 commit d5c3918

8 files changed

+29
-18
lines changed

third_party/blink/renderer/modules/locks/lock_manager.cc

+15-1
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,21 @@ class LockManager::LockRequestImpl final
236236
const char LockManager::kSupplementName[] = "LockManager";
237237

238238
// static
239-
LockManager* LockManager::locks(NavigatorBase& navigator) {
239+
LockManager* LockManager::locks(NavigatorBase& navigator,
240+
ExceptionState& exception_state) {
241+
ExecutionContext* context = navigator.GetExecutionContext();
242+
243+
auto* shared_storage_worklet_global_scope =
244+
DynamicTo<SharedStorageWorkletGlobalScope>(context);
245+
246+
if (shared_storage_worklet_global_scope &&
247+
!shared_storage_worklet_global_scope->add_module_finished()) {
248+
exception_state.ThrowDOMException(
249+
DOMExceptionCode::kNotAllowedError,
250+
"navigator.locks cannot be accessed during addModule().");
251+
return nullptr;
252+
}
253+
240254
auto* supplement = Supplement<NavigatorBase>::From<LockManager>(navigator);
241255
if (!supplement) {
242256
supplement = MakeGarbageCollected<LockManager>(navigator);

third_party/blink/renderer/modules/locks/lock_manager.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class LockManager final : public ScriptWrappable,
3636
static const char kSupplementName[];
3737

3838
// Web-exposed as navigator.locks
39-
static LockManager* locks(NavigatorBase&);
39+
static LockManager* locks(NavigatorBase&, ExceptionState&);
4040

4141
explicit LockManager(NavigatorBase&);
4242

third_party/blink/renderer/modules/locks/navigator_locks.idl

+1
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,6 @@
88
Exposed=Window,
99
ImplementedAs=LockManager
1010
] partial interface Navigator {
11+
[RaisesException]
1112
readonly attribute LockManager locks;
1213
};

third_party/blink/renderer/modules/locks/shared_storage_worklet_navigator_locks.idl

+1
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,6 @@
1010
Exposed=SharedStorageWorklet,
1111
ImplementedAs=LockManager
1212
] partial interface SharedStorageWorkletNavigator {
13+
[RaisesException]
1314
readonly attribute LockManager locks;
1415
};

third_party/blink/renderer/modules/locks/worker_navigator_locks.idl

+1
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,6 @@
88
Exposed=Worker,
99
ImplementedAs=LockManager
1010
] partial interface WorkerNavigator {
11+
[RaisesException]
1112
readonly attribute LockManager locks;
1213
};

third_party/blink/renderer/modules/shared_storage/shared_storage_worklet_global_scope.cc

-10
Original file line numberDiff line numberDiff line change
@@ -1054,16 +1054,6 @@ SharedStorageWorkletGlobalScope::interestGroups(
10541054
SharedStorageWorkletNavigator* SharedStorageWorkletGlobalScope::Navigator(
10551055
ScriptState* script_state,
10561056
ExceptionState& exception_state) {
1057-
if (!add_module_finished_) {
1058-
CHECK(!navigator_);
1059-
1060-
exception_state.ThrowDOMException(
1061-
DOMExceptionCode::kNotAllowedError,
1062-
"navigator cannot be accessed during addModule().");
1063-
1064-
return nullptr;
1065-
}
1066-
10671057
if (!navigator_) {
10681058
navigator_ = MakeGarbageCollected<SharedStorageWorkletNavigator>(
10691059
GetExecutionContext());

third_party/blink/renderer/modules/shared_storage/shared_storage_worklet_global_scope.h

+2
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ class MODULES_EXPORT SharedStorageWorkletGlobalScope final
147147
return permissions_policy_state_;
148148
}
149149

150+
bool add_module_finished() const { return add_module_finished_; }
151+
150152
private:
151153
void OnModuleScriptDownloaded(
152154
const KURL& script_source_url,

third_party/blink/renderer/modules/shared_storage/shared_storage_worklet_unittest.cc

+8-6
Original file line numberDiff line numberDiff line change
@@ -830,7 +830,8 @@ TEST_F(SharedStorageWorkletTest,
830830
AddModuleResult add_module_result = AddModule(/*script_content=*/R"(
831831
var expectedObjects = [
832832
"console",
833-
"crypto"
833+
"crypto",
834+
"navigator"
834835
];
835836
836837
var expectedFunctions = [
@@ -880,9 +881,10 @@ TEST_F(SharedStorageWorkletTest,
880881
console.log("Expected error:", e.message);
881882
}
882883
883-
// Verify that trying to access `navigator` would throw a custom error.
884+
// Verify that trying to access `navigator.locks` would throw a custom
885+
// error.
884886
try {
885-
navigator;
887+
navigator.locks;
886888
} catch (e) {
887889
console.log("Expected error:", e.message);
888890
}
@@ -904,9 +906,9 @@ TEST_F(SharedStorageWorkletTest,
904906
"'SharedStorageWorkletGlobalScope': sharedStorage cannot be "
905907
"accessed during addModule().");
906908
EXPECT_EQ(test_client_->observed_console_log_messages_[1],
907-
"Expected error: Failed to read the 'navigator' property from "
908-
"'SharedStorageWorkletGlobalScope': navigator cannot be accessed "
909-
"during addModule().");
909+
"Expected error: Failed to read the 'locks' property from "
910+
"'SharedStorageWorkletNavigator': navigator.locks cannot be "
911+
"accessed during addModule().");
910912
EXPECT_EQ(test_client_->observed_console_log_messages_[2],
911913
"Expected async error: Failed to execute 'interestGroups' on "
912914
"'SharedStorageWorkletGlobalScope': interestGroups() cannot be "

0 commit comments

Comments
 (0)