From b146687b75ecb53bfa71b467c57ccfcdcbd9cfe6 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Sun, 4 Oct 2026 12:56:02 +0800 Subject: [PATCH] fix(fetch): enforce Request header guards on fetch initializers --- moli-renderer-v8/src/network_host.rs | 2 +- .../src/network_host/fetch/input.rs | 16 +-- moli-renderer-v8/src/network_host/request.rs | 2 +- .../src/network_host/request/input.rs | 2 +- .../script_vm/tests/fetch_request_guard.rs | 71 ++++++++++++ moli-renderer-v8/src/script_vm/tests/mod.rs | 2 + .../src/worker/global_scope/fetch.rs | 16 +-- .../src/worker/global_scope/mod.rs | 8 +- .../src/worker/thread/tests/network.rs | 50 +++++++++ .../tests/fixtures/fetch-request-guard.js | 104 ++++++++++++++++++ 10 files changed, 252 insertions(+), 21 deletions(-) create mode 100644 moli-renderer-v8/src/script_vm/tests/fetch_request_guard.rs create mode 100644 moli-renderer-v8/tests/fixtures/fetch-request-guard.js diff --git a/moli-renderer-v8/src/network_host.rs b/moli-renderer-v8/src/network_host.rs index f2b3d67d65..8c4c835958 100644 --- a/moli-renderer-v8/src/network_host.rs +++ b/moli-renderer-v8/src/network_host.rs @@ -129,7 +129,7 @@ pub(in crate::network_host) use self::request::normalize_request_method; pub(crate) use self::request::request_constructor_callback; pub(crate) use self::request::{FetchArgumentError, RequestUrlError, convert_fetch_arguments}; pub(crate) use self::request::{ - mark_request_input_body_used_for_fetch, request_input_snapshot, + mark_request_input_body_used_for_fetch, request_headers_guard_for_mode, request_input_snapshot, try_resolve_request_constructor_url, try_resolve_request_constructor_url_for_base, try_resolve_request_constructor_url_for_child, }; diff --git a/moli-renderer-v8/src/network_host/fetch/input.rs b/moli-renderer-v8/src/network_host/fetch/input.rs index 3137d83738..4c4268d593 100644 --- a/moli-renderer-v8/src/network_host/fetch/input.rs +++ b/moli-renderer-v8/src/network_host/fetch/input.rs @@ -60,8 +60,11 @@ pub(super) fn parse_window_fetch_input<'s>( .or_else(|| moli_fetch::RequestMode::from_str(&inherited.mode).ok()) .unwrap_or(moli_fetch::RequestMode::Cors); validate_no_cors_method(request_mode, &method)?; - if request_mode == moli_fetch::RequestMode::NoCors { - headers = filter_headers_for_guard(&headers, HeadersGuard::RequestNoCors); + if init.headers_present || request_mode == moli_fetch::RequestMode::NoCors { + headers = filter_headers_for_guard( + &headers, + request_headers_guard_for_mode(request_mode.as_ref()), + ); } let credentials_mode = init .credentials_mode @@ -96,11 +99,10 @@ pub(super) fn parse_window_fetch_input<'s>( let init = parse_fetch_init(scope, args, 1)?; let request_mode = init.request_mode.unwrap_or(moli_fetch::RequestMode::Cors); validate_no_cors_method(request_mode, &init.method)?; - let headers = if request_mode == moli_fetch::RequestMode::NoCors { - filter_headers_for_guard(&init.headers, HeadersGuard::RequestNoCors) - } else { - init.headers - }; + let headers = filter_headers_for_guard( + &init.headers, + request_headers_guard_for_mode(request_mode.as_ref()), + ); let credentials_mode = init .credentials_mode .unwrap_or(moli_fetch::RequestCredentialsMode::SameOrigin); diff --git a/moli-renderer-v8/src/network_host/request.rs b/moli-renderer-v8/src/network_host/request.rs index 5136618164..1c95b33a1a 100644 --- a/moli-renderer-v8/src/network_host/request.rs +++ b/moli-renderer-v8/src/network_host/request.rs @@ -16,7 +16,7 @@ pub(crate) use self::init::{parse_fetch_init, request_object_credentials_mode}; pub(crate) use self::init::{parse_request_redirect_mode_label, request_redirect_mode_label}; pub(in crate::network_host) use self::input::normalize_request_method; pub(crate) use self::input::{ - mark_request_input_body_used_for_fetch, request_input_snapshot, + mark_request_input_body_used_for_fetch, request_headers_guard_for_mode, request_input_snapshot, try_resolve_request_constructor_url, try_resolve_request_constructor_url_for_base, try_resolve_request_constructor_url_for_child, }; diff --git a/moli-renderer-v8/src/network_host/request/input.rs b/moli-renderer-v8/src/network_host/request/input.rs index d679dedd8d..fe7b7545f9 100644 --- a/moli-renderer-v8/src/network_host/request/input.rs +++ b/moli-renderer-v8/src/network_host/request/input.rs @@ -32,7 +32,7 @@ pub(super) fn request_method_allows_body(method: &str) -> bool { !matches!(method, "GET" | "HEAD") } -pub(super) fn request_headers_guard_for_mode(mode: &str) -> HeadersGuard { +pub(crate) fn request_headers_guard_for_mode(mode: &str) -> HeadersGuard { if mode == "no-cors" { HeadersGuard::RequestNoCors } else { diff --git a/moli-renderer-v8/src/script_vm/tests/fetch_request_guard.rs b/moli-renderer-v8/src/script_vm/tests/fetch_request_guard.rs new file mode 100644 index 0000000000..7ec5c93aea --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/fetch_request_guard.rs @@ -0,0 +1,71 @@ +use super::*; + +#[tokio::test(flavor = "current_thread")] +async fn fetch_request_initializers_use_request_header_guards() { + let base = "https://fetch-guard.test"; + let loader = static_http_loader(std::iter::empty::()); + let mut vm = + new_storage_page_task_executor_test_vm_with_loader(&format!("{base}/page.html"), &loader); + vm.set_fetch_subresource_interception(true, Some(crate::types::SubresourceResourceType::Fetch)); + vm.eval(&format!( + "{}\nglobalThis.guardResult = null; fetchRequestGuardProbe('{base}', false).then(value => {{ guardResult = value; }}, error => {{ guardResult = {{ error: String(error) }}; }});", + include_str!("../../../tests/fixtures/fetch-request-guard.js"), + )).unwrap(); + let mut requests = 0; + 'requests: for _ in 0..40 { + let pending = loop { + if vm.eval("guardResult !== null").unwrap() == "true" { + break 'requests; + } + let pending = vm.take_pending_subresource_fetch_infos(); + if !pending.is_empty() { + break pending; + } + wait_for_one_selected_page_task_executor_test_turn(&mut vm, &loader) + .await + .expect("header guard probe should advance to its next fetch or result"); + }; + assert_eq!(pending.len(), 1, "request {requests}"); + let request = &pending[0]; + assert_eq!(request.url.path(), "/echo"); + let body = + serde_json::json!({"headers": request.request_headers.to_byte_strings()}).to_string(); + let head = moli_fetch::ResponseHead { + final_url: request.url.clone(), + status: 200, + headers: vec![("content-type".to_owned(), b"application/json".to_vec())], + request_cookie_report: None, + cookie_set_reports: Vec::new(), + redirected: false, + redirect_chain: Vec::new(), + from_cache: false, + negotiated_http_version: None, + }; + vm.complete_async_subresource_fetch(crate::types::AsyncSubresourceFetchCompletion { + response_filter: None, + skip_fetch_security_validation: false, + internal_id: request.internal_id, + request_url: request.url.clone(), + request_method: request.method.clone(), + request_headers: request.request_headers.clone(), + request_body: None, + response_status_text: None, + network_error_text: None, + result: Ok( + crate::protocol_types::NavigationResponse::from_head_and_body( + head, + body.clone(), + body.into_bytes(), + ), + ) + .into(), + }) + .unwrap(); + requests += 1; + } + let result: serde_json::Value = + serde_json::from_str(&vm.eval("JSON.stringify(guardResult)").unwrap()).unwrap(); + assert_eq!(requests, 33); + assert_eq!(result["state"], "pass", "{result}"); + assert_eq!(result["checks"].as_array().unwrap().len(), 1046); +} diff --git a/moli-renderer-v8/src/script_vm/tests/mod.rs b/moli-renderer-v8/src/script_vm/tests/mod.rs index c36aeb02e0..7ab7f94d94 100644 --- a/moli-renderer-v8/src/script_vm/tests/mod.rs +++ b/moli-renderer-v8/src/script_vm/tests/mod.rs @@ -2194,3 +2194,5 @@ mod media_device_interfaces; mod dom_rect_factory_descriptors; mod dom_rect_structured_clone; + +mod fetch_request_guard; diff --git a/moli-renderer-v8/src/worker/global_scope/fetch.rs b/moli-renderer-v8/src/worker/global_scope/fetch.rs index 9f7c8042bb..f380227665 100644 --- a/moli-renderer-v8/src/worker/global_scope/fetch.rs +++ b/moli-renderer-v8/src/worker/global_scope/fetch.rs @@ -1895,8 +1895,11 @@ pub(in crate::worker) fn resolve_worker_fetch_input<'s>( .or_else(|| moli_fetch::RequestMode::from_str(&inherited.mode).ok()) .unwrap_or(moli_fetch::RequestMode::Cors); validate_worker_no_cors_method(request_mode, &method)?; - let headers = if request_mode == moli_fetch::RequestMode::NoCors { - filter_headers_for_guard(&headers, HeadersGuard::RequestNoCors) + let headers = if init.headers_present || request_mode == moli_fetch::RequestMode::NoCors { + filter_headers_for_guard( + &headers, + request_headers_guard_for_mode(request_mode.as_ref()), + ) } else { headers }; @@ -1940,11 +1943,10 @@ pub(in crate::worker) fn resolve_worker_fetch_input<'s>( let init = parse_fetch_init(scope, args, 1)?; let request_mode = init.request_mode.unwrap_or(moli_fetch::RequestMode::Cors); validate_worker_no_cors_method(request_mode, &init.method)?; - let headers = if request_mode == moli_fetch::RequestMode::NoCors { - filter_headers_for_guard(&init.headers, HeadersGuard::RequestNoCors) - } else { - init.headers - }; + let headers = filter_headers_for_guard( + &init.headers, + request_headers_guard_for_mode(request_mode.as_ref()), + ); let credentials_mode = init .credentials_mode .unwrap_or(RequestCredentialsMode::SameOrigin); diff --git a/moli-renderer-v8/src/worker/global_scope/mod.rs b/moli-renderer-v8/src/worker/global_scope/mod.rs index 2050ece39a..35a1a4e718 100644 --- a/moli-renderer-v8/src/worker/global_scope/mod.rs +++ b/moli-renderer-v8/src/worker/global_scope/mod.rs @@ -58,7 +58,7 @@ use crate::context_bootstrap::{ use crate::network::loads::{ResourceLoadDisposition, ResourceLoadKind, ResourceLoadLease}; use crate::network_host::{ ABORTED_ERROR_TEXT, BLOCKED_BY_CLIENT_ERROR_TEXT, FAILED_ERROR_TEXT, - FetchResponseSecurityViolation, HeadersGuard, PreparedXhrSendBody, XHR_ABORTED_SLOT, + FetchResponseSecurityViolation, PreparedXhrSendBody, XHR_ABORTED_SLOT, XHR_ACTIVE_INTERNAL_ID_SLOT, XHR_ASYNC_SLOT, XHR_METHOD_SLOT, XHR_OPEN_GENERATION_SLOT, XHR_READY_STATE_SLOT, XHR_SEND_FLAG_SLOT, XHR_TIMEOUT_SLOT, XHR_TIMEOUT_START_MS_SLOT, XHR_TIMEOUT_TIMER_SLOT, XHR_URL_SLOT, XHR_WITH_CREDENTIALS_SLOT, @@ -75,9 +75,9 @@ use crate::network_host::{ fetch_browser_subresource_raw_stream_with_preflight_headers_and_network_metadata, fetch_browser_subresource_with_preflight_headers_and_network_metadata, filter_cors_exposed_response_headers, filter_headers_for_guard, is_cors_policy_failure_message, - local_url_response_result, parse_fetch_init, request_input_snapshot, - request_object_credentials_mode, reset_xhr_response_for_request_error, resolve_context_url, - set_xhr_state_bool, set_xhr_state_number, throw_synchronous_xhr_failure, + local_url_response_result, parse_fetch_init, request_headers_guard_for_mode, + request_input_snapshot, request_object_credentials_mode, reset_xhr_response_for_request_error, + resolve_context_url, set_xhr_state_bool, set_xhr_state_number, throw_synchronous_xhr_failure, validate_fetch_response_security_policy, validate_fetch_response_security_policy_with_body_classified, xhr_author_request_headers, xhr_dispatch_progress_event, xhr_ensure_send_allowed, xhr_state_bool_property, diff --git a/moli-renderer-v8/src/worker/thread/tests/network.rs b/moli-renderer-v8/src/worker/thread/tests/network.rs index 9d38d2959c..f542379bfa 100644 --- a/moli-renderer-v8/src/worker/thread/tests/network.rs +++ b/moli-renderer-v8/src/worker/thread/tests/network.rs @@ -22,3 +22,53 @@ mod opfs_storage; mod websockets; mod worker_globals; mod worker_storage_security; + +#[tokio::test] +async fn worker_fetch_request_initializers_use_request_header_guards() { + ensure_v8(); + let source = format!( + "{}\nonmessage = async () => {{ const result = await fetchRequestGuardProbe('https://fetch-guard.test', false); postMessage(result); close(); }};", + include_str!("../../../../tests/fixtures/fetch-request-guard.js"), + ); + let mut handle = spawn_worker_with_request_client_and_network_policy( + source, + "https://fetch-guard.test/worker.js".into(), + ResourceRequestClient::new(&FetchConfig::default()).expect("worker fetch loader"), + WorkerNetworkPolicy::default(), + ); + handle.set_fetch_subresource_interception(true, Some(SubresourceResourceType::Fetch)); + handle.post_message(serialize_test_string("go")); + let mut requests = 0; + let result = loop { + let message = timeout(TIMEOUT, handle.recv()) + .await + .expect("worker response timeout") + .expect("worker channel closed"); + match message { + WorkerToParentMessage::PendingSubresourceFetch(pending) => { + assert_eq!(pending.info.url.path(), "/echo"); + requests += 1; + assert!(requests <= 33); + let body = + serde_json::json!({"headers": pending.info.request_headers.to_byte_strings()}) + .to_string(); + let request = + pending_worker_fetch_continue(pending.fetch_id, requests, &pending.info, false); + handle.fulfill_pending_fetch( + request, + 200, + vec![("content-type".to_owned(), b"application/json".to_vec())], + RendererSyntheticResponseBody::from_bytes(body.into_bytes()), + ); + } + WorkerToParentMessage::Post(payload) => break stringify_payload(&payload), + WorkerToParentMessage::SubresourceNetwork(_) + | WorkerToParentMessage::SubresourceContinue(_) => {} + other => panic!("unexpected worker response: {other:?}"), + } + }; + let result: serde_json::Value = serde_json::from_str(&result).unwrap(); + assert_eq!(requests, 33); + assert_eq!(result["state"], "pass", "{result}"); + assert_eq!(result["checks"].as_array().unwrap().len(), 1046); +} diff --git a/moli-renderer-v8/tests/fixtures/fetch-request-guard.js b/moli-renderer-v8/tests/fixtures/fetch-request-guard.js new file mode 100644 index 0000000000..61b97f0f4f --- /dev/null +++ b/moli-renderer-v8/tests/fixtures/fetch-request-guard.js @@ -0,0 +1,104 @@ +async function fetchRequestGuardProbe(base, transport = true) { + const checks = []; + let requests = 0; + const check = (label, actual, wanted) => checks.push({label, actual, wanted, pass: actual === wanted}); + const forbidden = [ + ['Accept-Charset', 'author-charset'], ['Accept-Encoding', 'author-encoding'], + ['Access-Control-Request-Headers', 'author-header'], ['Access-Control-Request-Method', 'AUTHOR'], + ['Connection', 'author-connection'], ['Content-Length', '42'], + ['Cookie', 'author_cookie=bad'], ['Cookie2', 'author_cookie2=bad'], + ['Date', 'author-date'], ['DNT', '4'], ['Host', 'forged.invalid'], + ['Origin', 'https://forged.invalid'], ['Referer', 'https://forged.invalid/'], + ['Set-Cookie', 'author_response=bad'], ['TE', 'author-te'], + ['Trailer', 'author-trailer'], ['Upgrade', 'author-upgrade'], ['Via', 'author-via'], + ['Proxy-Probe', 'author-proxy'], ['Sec-Probe', 'author-sec'] + ]; + const pairs = [...forbidden, + ['Accept', 'text/plain'], ['Content-Language', 'en'], ['Content-Type', 'text/plain'], + ['X-Allowed', 'kept'], ['X-Cookie', 'ordinary'], ['Authorization', 'Bearer ordinary'], + ['X-HTTP-Method-Override', 'GETTRACE'], ['X-HTTP-Method', '\",TRACE\",'], + ['X-Method-Override', 'GET'], ['x-method-override', 'TRACE'] + ]; + const snapshot = headers => JSON.stringify(Array.from(new Headers(headers).entries())); + async function send(label, input, init, mode, expected, initializer) { + const originalRequest = typeof input === 'string' ? null : snapshot(input.headers); + const originalInit = initializer === undefined ? null : snapshot(initializer); + requests++; + try { + const response = await fetch(input, init); + check(label + '/status', response.status, 200); + const observed = new Headers((await response.json()).headers); + for (const [name, value] of forbidden) { + check(label + '/' + name + '/forbidden', observed.get(name) === value, false); + } + for (const [name, value] of Object.entries(expected)) check(label + '/' + name, observed.get(name), value); + if (transport) { + check(label + '/host', observed.get('Host'), new URL(base).host); + check(label + '/cookie', (observed.get('Cookie') || '').includes('guard_real=1'), true); + check(label + '/fetch-mode', observed.get('Sec-Fetch-Mode'), mode); + check(label + '/referrer', (observed.get('Referer') || '').startsWith(base), true); + } + } catch (error) { + check(label + '/fetch', String(error), 'successful response'); + } + if (originalRequest !== null) check(label + '/input-unchanged', snapshot(input.headers), originalRequest); + if (originalInit !== null) check(label + '/initializer-unchanged', snapshot(initializer), originalInit); + } + for (const mode of ['cors', 'same-origin', 'no-cors']) { + for (const inputKind of ['url', 'request-override', 'request-inherit-mode']) { + for (const form of ['record', 'pairs', 'headers']) { + const label = [mode, inputKind, form].join('/'); + const headers = form === 'record' ? Object.fromEntries(pairs) + : form === 'headers' ? new Headers(pairs) : pairs.map(pair => pair.slice()); + const url = base + '/echo?case=' + encodeURIComponent(label); + const input = inputKind === 'url' ? url : new Request(url, { + mode: inputKind === 'request-inherit-mode' ? mode : 'cors', + headers: {'X-Inherited': 'original'} + }); + const init = {headers}; + if (inputKind === 'request-override' || inputKind === 'url' && mode !== 'cors') init.mode = mode; + const allowed = value => mode === 'no-cors' ? null : value; + await send(label, input, init, mode, { + 'Accept': 'text/plain', 'Content-Language': 'en', 'Content-Type': 'text/plain', + 'X-Allowed': allowed('kept'), 'X-Cookie': allowed('ordinary'), + 'Authorization': allowed('Bearer ordinary'), 'X-Inherited': null, + 'X-HTTP-Method-Override': allowed('GETTRACE'), 'X-HTTP-Method': allowed('\",TRACE\",'), + // A Headers initializer has already combined these two values. + // Record and sequence initializers validate each append separately. + 'X-Method-Override': allowed(form === 'headers' ? null : 'GET') + }, headers); + } + } + for (const emptyInit of [false, true]) { + const label = mode + '/inherited/' + emptyInit; + const input = new Request(base + '/echo?case=' + encodeURIComponent(label), { + mode, headers: {'X-Inherited': 'original', 'Accept': 'text/plain', 'Cookie': 'author_cookie=bad'} + }); + await send(label, input, emptyInit ? {} : undefined, mode, { + 'Accept': 'text/plain', 'X-Inherited': mode === 'no-cors' ? null : 'original' + }); + } + } + for (const inputKind of ['url', 'request']) { + const url = base + '/must-not-fetch'; + const input = inputKind === 'url' ? url : new Request(url); + for (const kind of ['getter', 'stringification', 'non-byte', 'invalid-value']) { + const label = inputKind + '/conversion/' + kind; + const token = {label}; + const log = []; + const headers = kind === 'getter' ? {get Cookie() {log.push('cookie'); throw token;}} + : kind === 'stringification' ? [['Cookie', {toString() {log.push('cookie'); throw token;}}]] + : {Cookie: kind === 'non-byte' ? '\u0100' : 'invalid\r\nvalue'}; + let synchronous = false; + let promise; + try { promise = fetch(input, {headers}); } + catch (error) { synchronous = true; promise = Promise.reject(error); } + check(label + '/async', synchronous, false); + await promise.then(() => check(label + '/rejection', 'fulfilled', 'rejected'), error => { + check(label + '/rejection', kind === 'getter' || kind === 'stringification' ? error === token : error instanceof TypeError, true); + }); + if (kind === 'getter' || kind === 'stringification') check(label + '/conversion', log.join(','), 'cookie'); + } + } + return {state: checks.every(entry => entry.pass) ? 'pass' : 'fail', requests, checks}; +}