diff --git a/moli-renderer-v8/src/network_host/async_fetch.rs b/moli-renderer-v8/src/network_host/async_fetch.rs index 20dc0bc22a..cd139ff787 100644 --- a/moli-renderer-v8/src/network_host/async_fetch.rs +++ b/moli-renderer-v8/src/network_host/async_fetch.rs @@ -548,6 +548,7 @@ async fn run_cors_preflight_if_needed( preflight_request_headers, preflight_response.status, &preflight_response.headers, + request.use_cors_preflight(), )?; } Ok(()) diff --git a/moli-renderer-v8/src/network_host/response/cors.rs b/moli-renderer-v8/src/network_host/response/cors.rs index 1106419a57..cd7b08c2a4 100644 --- a/moli-renderer-v8/src/network_host/response/cors.rs +++ b/moli-renderer-v8/src/network_host/response/cors.rs @@ -446,6 +446,7 @@ pub(crate) fn validate_cors_preflight_response( request_headers: &[(String, String)], response_status: u16, response_headers: &[(String, String)], + use_cors_preflight: bool, ) -> Result<(), String> { if !(200..300).contains(&response_status) { return Err(format!( @@ -454,20 +455,30 @@ pub(crate) fn validate_cors_preflight_response( } validate_cors_response_for_origin(origin, response_headers, credentials_mode)?; + // Parse both complete lists before checking permissions, including for + // safelisted methods and requests without unsafe header names. + let mut allow_methods = + parse_cors_preflight_allowlist(response_headers, "Access-Control-Allow-Methods")?; + let allow_headers = + parse_cors_preflight_allowlist(response_headers, "Access-Control-Allow-Headers")?; + if allow_methods.is_none() && use_cors_preflight { + allow_methods = Some(vec![requested_method.to_owned()]); + } + let wildcard_allowed = credentials_mode != RequestCredentialsMode::Include; + if !moli_fetch::is_cors_safelisted_method(requested_method) { - let Some(allow_methods) = - response_header_value(response_headers, "access-control-allow-methods") - else { + let Some(allow_methods) = allow_methods else { return Err(format!( "CORS preflight failed: no Access-Control-Allow-Methods for {requested_method}" )); }; - if !comma_separated_tokens(&allow_methods) + if !allow_methods .iter() - .any(|method| method == requested_method) + .any(|method| method == requested_method || (wildcard_allowed && method == "*")) { return Err(format!( - "CORS preflight failed: Access-Control-Allow-Methods `{allow_methods}` does not allow {requested_method}" + "CORS preflight failed: Access-Control-Allow-Methods `{}` does not allow {requested_method}", + allow_methods.join(",") )); } } @@ -477,22 +488,24 @@ pub(crate) fn validate_cors_preflight_response( return Ok(()); } - let Some(allow_headers) = - response_header_value(response_headers, "access-control-allow-headers") - else { + let Some(allow_headers) = allow_headers else { return Err(format!( "CORS preflight failed: no Access-Control-Allow-Headers for {}", unsafe_header_names.join(",") )); }; - let allowed_header_names = comma_separated_tokens(&allow_headers) - .into_iter() - .map(|name| name.to_ascii_lowercase()) - .collect::>(); + let wildcard_headers = wildcard_allowed && allow_headers.iter().any(|name| name == "*"); for header_name in unsafe_header_names { - if !allowed_header_names.iter().any(|name| name == &header_name) { + // Authorization is a CORS non-wildcard request-header name, so it + // always needs an explicit, case-insensitive match. + if !allow_headers + .iter() + .any(|name| name.eq_ignore_ascii_case(&header_name)) + && (!wildcard_headers || header_name == "authorization") + { return Err(format!( - "CORS preflight failed: Access-Control-Allow-Headers `{allow_headers}` does not allow {header_name}" + "CORS preflight failed: Access-Control-Allow-Headers `{}` does not allow {header_name}", + allow_headers.join(",") )); } } @@ -553,13 +566,32 @@ pub(crate) fn filter_cors_exposed_response_headers( .collect() } -fn comma_separated_tokens(value: &str) -> Vec { - value - .split(',') - .map(str::trim) - .filter(|token| !token.is_empty()) - .map(str::to_owned) - .collect() +fn parse_cors_preflight_allowlist( + headers: &[(String, String)], + name: &str, +) -> Result>, String> { + let values = response_header_values(headers, name); + if values.is_empty() { + return Ok(None); + } + let mut tokens = Vec::new(); + for value in values { + for token in value.split(',') { + let token = token.trim_matches([' ', '\t']); + if token.is_empty() { + continue; + } + // Both method and field-name use HTTP token syntax. Preserve case + // because method permissions require an exact match. + if http::Method::from_bytes(token.as_bytes()).is_err() { + return Err(format!( + "CORS preflight failed: invalid {name} value `{value}`" + )); + } + tokens.push(token.to_owned()); + } + } + Ok(Some(tokens)) } fn is_forbidden_response_header_name(name: &str) -> bool { @@ -809,6 +841,7 @@ mod tests { &request_headers, 204, &response_headers, + false, ), Ok(()) ); @@ -833,6 +866,7 @@ mod tests { &[("Content-Type".to_owned(), "custom/type".to_owned())], 200, &response_headers, + false, ) .unwrap_or_else(|error| { panic!( @@ -860,11 +894,188 @@ mod tests { &[("Content-Type".to_owned(), "custom/type".to_owned())], 200, &response_headers, + false, ) .expect_err("unsafelisted PUT preflight should require Access-Control-Allow-Methods"); assert!(error.contains("no Access-Control-Allow-Methods for PUT")); } + fn preflight_permissions( + method: &str, + request_headers: &[(&str, &str)], + permissions: &[(&str, &str)], + credentials_mode: RequestCredentialsMode, + ) -> Result<(), String> { + let request_headers = request_headers + .iter() + .map(|(name, value)| ((*name).to_owned(), (*value).to_owned())) + .collect::>(); + let mut response_headers = vec![ + ( + "Access-Control-Allow-Origin".to_owned(), + "https://origin.test".to_owned(), + ), + ( + "Access-Control-Allow-Credentials".to_owned(), + "true".to_owned(), + ), + ]; + response_headers.extend( + permissions + .iter() + .map(|(name, value)| ((*name).to_owned(), (*value).to_owned())), + ); + let origin = WebOrigin::from_url(&url("https://origin.test/page")); + validate_cors_preflight_response( + origin.ascii_serialization(), + credentials_mode, + method, + &request_headers, + 204, + &response_headers, + false, + ) + } + + #[test] + fn cors_preflight_permissions_combine_fields_without_folding_method_case() { + let permissions = [ + ("Access-Control-Allow-Methods", "POST,,"), + ("access-control-allow-methods", "\t patcH, \t"), + ("Access-Control-Allow-Headers", "X-First"), + ("ACCESS-CONTROL-ALLOW-HEADERS", ",\t X-SECOND,,"), + ]; + for credentials in [ + RequestCredentialsMode::Omit, + RequestCredentialsMode::Include, + ] { + let headers = [("x-first", "1"), ("x-second", "2")]; + assert_eq!( + preflight_permissions("patcH", &headers, &permissions, credentials), + Ok(()) + ); + assert!(preflight_permissions("PATCH", &headers, &permissions, credentials).is_err()); + } + } + + #[test] + fn cors_preflight_permissions_validate_both_complete_lists_before_safelists() { + for field in [ + "Access-Control-Allow-Methods", + "Access-Control-Allow-Headers", + ] { + for invalid in [ + "Bad value", + "\"GET\"", + "GET:POST", + "GET;POST", + "GET\u{00a0}", + "\u{000b}GET", + "GET\r\n", + "GÉT", + ] { + let fields = [(field, "GET, X-Test"), (field, invalid)]; + assert!( + preflight_permissions("GET", &[], &fields, RequestCredentialsMode::Omit) + .is_err(), + "a later malformed {field} must reject even a safelisted request: {invalid:?}" + ); + assert!( + preflight_permissions( + "GET", + &[("X-Test", "1")], + &fields, + RequestCredentialsMode::Omit + ) + .is_err(), + "an earlier matching token must not hide a malformed {field}: {invalid:?}" + ); + } + } + } + + #[test] + fn cors_preflight_permissions_wildcards_respect_credentials_and_authorization() { + let wildcards = [ + ("Access-Control-Allow-Methods", "*"), + ("Access-Control-Allow-Headers", "*"), + ]; + for credentials in [ + RequestCredentialsMode::Omit, + RequestCredentialsMode::SameOrigin, + RequestCredentialsMode::Include, + ] { + assert_eq!( + preflight_permissions("PUT", &[("X-Test", "1")], &wildcards, credentials).is_ok(), + credentials != RequestCredentialsMode::Include + ); + assert_eq!( + preflight_permissions("*", &[("*", "1")], &wildcards, credentials), + Ok(()) + ); + assert!( + preflight_permissions( + "POST", + &[("aUtHoRiZaTiOn", "secret")], + &wildcards, + credentials + ) + .is_err() + ); + assert_eq!( + preflight_permissions( + "POST", + &[("Authorization", "secret")], + &[ + ("Access-Control-Allow-Headers", "*"), + ("Access-Control-Allow-Headers", "AUTHORIZATION"), + ], + credentials + ), + Ok(()) + ); + } + } + + #[test] + fn cors_preflight_permissions_accept_http_tokens_and_empty_lists() { + let token = "!#$%&'*+-.^_`|~0123456789AZaz"; + assert_eq!( + preflight_permissions( + token, + &[(token, "1")], + &[ + ("Access-Control-Allow-Methods", token), + ("Access-Control-Allow-Headers", token), + ], + RequestCredentialsMode::Include + ), + Ok(()) + ); + for empty in ["", " \t ", ",, \t,"] { + let fields = [ + ("Access-Control-Allow-Methods", empty), + ("Access-Control-Allow-Headers", empty), + ]; + assert_eq!( + preflight_permissions("GET", &[], &fields, RequestCredentialsMode::Omit), + Ok(()) + ); + assert!( + preflight_permissions("PUT", &[], &fields, RequestCredentialsMode::Omit).is_err() + ); + assert!( + preflight_permissions( + "GET", + &[("X-Test", "1")], + &fields, + RequestCredentialsMode::Omit + ) + .is_err() + ); + } + } + #[test] fn cors_exposed_headers_keep_safelisted_and_explicit_names() { let headers = vec![ diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/cors_preflight.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/cors_preflight.rs new file mode 100644 index 0000000000..521ec27f70 --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/cors_preflight.rs @@ -0,0 +1,392 @@ +use super::*; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; + +struct PreflightCase { + label: &'static str, + method: &'static str, + headers: &'static [(&'static str, &'static str)], + permissions: &'static [(&'static str, &'static str)], + credentials: bool, + upload_listener: bool, + allowed: bool, +} + +fn permission_cases() -> Vec { + const DEFAULT: PreflightCase = PreflightCase { + label: "", + method: "PUT", + headers: &[("X-Test", "1")], + permissions: &[], + credentials: false, + upload_listener: false, + allowed: true, + }; + vec![ + PreflightCase { + label: "duplicate-fields", + permissions: &[ + ("Access-Control-Allow-Methods", "POST"), + ("access-control-allow-methods", "PUT"), + ("Access-Control-Allow-Headers", "x-other"), + ("ACCESS-CONTROL-ALLOW-HEADERS", "X-Test"), + ], + ..DEFAULT + }, + PreflightCase { + label: "wildcards", + permissions: &[ + ("Access-Control-Allow-Methods", "*"), + ("Access-Control-Allow-Headers", "*"), + ], + ..DEFAULT + }, + PreflightCase { + label: "credentialed-method-wildcard", + permissions: &[ + ("Access-Control-Allow-Methods", "*"), + ("Access-Control-Allow-Headers", "X-Test"), + ], + credentials: true, + allowed: false, + ..DEFAULT + }, + PreflightCase { + label: "credentialed-header-wildcard", + permissions: &[ + ("Access-Control-Allow-Methods", "PUT"), + ("Access-Control-Allow-Headers", "*"), + ], + credentials: true, + allowed: false, + ..DEFAULT + }, + PreflightCase { + label: "credentialed-explicit-permissions", + permissions: &[ + ("Access-Control-Allow-Methods", "*, PUT"), + ("Access-Control-Allow-Headers", "*, x-test"), + ], + credentials: true, + ..DEFAULT + }, + PreflightCase { + label: "credentialed-literal-stars", + method: "*", + headers: &[("*", "1")], + permissions: &[ + ("Access-Control-Allow-Methods", "*"), + ("Access-Control-Allow-Headers", "*"), + ], + credentials: true, + ..DEFAULT + }, + PreflightCase { + label: "authorization-needs-explicit-permission", + method: "POST", + headers: &[("Authorization", "secret")], + permissions: &[("Access-Control-Allow-Headers", "*")], + allowed: false, + ..DEFAULT + }, + PreflightCase { + label: "authorization-in-second-field", + method: "POST", + headers: &[("aUtHoRiZaTiOn", "secret")], + permissions: &[ + ("Access-Control-Allow-Headers", "*"), + ("Access-Control-Allow-Headers", "AUTHORIZATION"), + ], + credentials: true, + ..DEFAULT + }, + PreflightCase { + label: "case-sensitive-method", + method: "patcH", + permissions: &[ + ("Access-Control-Allow-Methods", "patcH"), + ("Access-Control-Allow-Headers", "x-TEST"), + ], + ..DEFAULT + }, + PreflightCase { + label: "method-case-mismatch", + method: "patcH", + permissions: &[ + ("Access-Control-Allow-Methods", "PATCH"), + ("Access-Control-Allow-Headers", "x-test"), + ], + allowed: false, + ..DEFAULT + }, + PreflightCase { + label: "empty-elements-and-http-whitespace", + permissions: &[ + ("Access-Control-Allow-Methods", ",\t PUT, ,"), + ("Access-Control-Allow-Headers", "\t, X-Test,\t,"), + ], + ..DEFAULT + }, + PreflightCase { + label: "missing-method-permission", + permissions: &[("Access-Control-Allow-Headers", "x-test")], + allowed: false, + ..DEFAULT + }, + PreflightCase { + label: "safelisted-method-without-permission", + method: "GET", + permissions: &[("Access-Control-Allow-Headers", "x-test")], + ..DEFAULT + }, + PreflightCase { + label: "malformed-method-after-matching-field", + permissions: &[ + ("Access-Control-Allow-Methods", "PUT"), + ("Access-Control-Allow-Methods", "Bad value"), + ("Access-Control-Allow-Headers", "X-Test"), + ], + allowed: false, + ..DEFAULT + }, + PreflightCase { + label: "malformed-header-after-matching-field", + permissions: &[ + ("Access-Control-Allow-Methods", "PUT"), + ("Access-Control-Allow-Headers", "X-Test"), + ("Access-Control-Allow-Headers", "Bad value"), + ], + allowed: false, + ..DEFAULT + }, + PreflightCase { + label: "safelisted-method-with-malformed-method-list", + method: "GET", + permissions: &[ + ("Access-Control-Allow-Methods", "Bad value"), + ("Access-Control-Allow-Headers", "X-Test"), + ], + allowed: false, + ..DEFAULT + }, + PreflightCase { + label: "no-unsafe-headers-with-malformed-header-list", + headers: &[], + permissions: &[ + ("Access-Control-Allow-Methods", "PUT"), + ("Access-Control-Allow-Headers", "Bad value"), + ], + allowed: false, + ..DEFAULT + }, + PreflightCase { + label: "upload-flag-with-missing-method-list", + headers: &[], + upload_listener: true, + ..DEFAULT + }, + PreflightCase { + label: "upload-flag-with-empty-method-list", + headers: &[], + permissions: &[("Access-Control-Allow-Methods", "")], + upload_listener: true, + allowed: false, + ..DEFAULT + }, + PreflightCase { + label: "credentialed-upload-with-missing-method-list", + headers: &[], + upload_listener: true, + credentials: true, + ..DEFAULT + }, + PreflightCase { + label: "upload-flag-still-requires-header-permission", + upload_listener: true, + allowed: false, + ..DEFAULT + }, + PreflightCase { + label: "upload-flag-with-different-method-list", + headers: &[], + permissions: &[("Access-Control-Allow-Methods", "GET")], + upload_listener: true, + allowed: false, + ..DEFAULT + }, + ] +} + +async fn check_preflight_permissions(worker: bool) { + for api in ["fetch", "xhr", "sync-xhr"] { + let cases = std::sync::Arc::new( + permission_cases() + .into_iter() + .filter(|case| api != "fetch" || !case.upload_listener) + .collect::>(), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let base = format!("http://{}/", listener.local_addr().unwrap()); + let server_cases = cases.clone(); + let (stop_tx, mut stop_rx) = tokio::sync::oneshot::channel::<()>(); + let server = tokio::spawn(async move { + let mut observed = Vec::new(); + loop { + let mut socket = tokio::select! { + accepted = listener.accept() => accepted.unwrap().0, + _ = &mut stop_rx => break, + }; + let mut head = Vec::new(); + let mut byte = [0; 1]; + while !head.ends_with(b"\r\n\r\n") { + assert!(head.len() < 8192); + assert_eq!(socket.read(&mut byte).await.unwrap(), 1); + head.push(byte[0]); + } + let head = String::from_utf8(head).unwrap(); + let mut request_line = head.lines().next().unwrap().split_whitespace(); + let method = request_line.next().unwrap().to_owned(); + let index: usize = request_line + .next() + .unwrap() + .trim_start_matches('/') + .parse() + .unwrap(); + let length = head + .lines() + .find_map(|line| { + let (name, value) = line.split_once(':')?; + name.eq_ignore_ascii_case("content-length") + .then(|| value.trim().parse::().unwrap()) + }) + .unwrap_or(0); + socket.read_exact(&mut vec![0; length]).await.unwrap(); + let mut response = "HTTP/1.1 200 OK\r\nAccess-Control-Allow-Origin: http://origin.test\r\nAccess-Control-Allow-Credentials: true\r\nAccess-Control-Max-Age: 0\r\nContent-Length: 2\r\nConnection: close\r\n".to_owned(); + if method == "OPTIONS" { + for (name, value) in server_cases[index].permissions { + response.push_str(&format!("{name}: {value}\r\n")); + } + let lower_head = head.to_ascii_lowercase(); + assert!( + !lower_head.contains("\r\nauthorization:"), + "OPTIONS must not carry credentials" + ); + assert!(lower_head.contains(&format!( + "\r\naccess-control-request-method: {}\r\n", + server_cases[index].method.to_ascii_lowercase() + ))); + } + observed.push((index, method)); + response.push_str("\r\nok"); + socket.write_all(response.as_bytes()).await.unwrap(); + } + observed + }); + let mut config = moli_fetch::FetchConfig::default(); + config.set_http_no_proxy(Some("*".to_owned())); + let loader = ResourceRequestClient::new(&config).unwrap(); + let mut vm = new_page_task_executor_test_vm_with_loader("http://origin.test/page", &loader); + let inputs = cases + .iter() + .map(|case| { + serde_json::json!({ + "label": case.label, "method": case.method, "headers": case.headers, + "credentials": case.credentials, "upload": case.upload_listener, + }) + }) + .collect::>(); + let probe = format!( + r#" + (async () => {{ + const cases = {}, results = []; + const api = {api:?}, base = {base:?}; + for (const [index, item] of cases.entries()) {{ + const url = base + index; + let allowed = false; + if (api === 'fetch') {{ + try {{ + const response = await fetch(url, {{method: item.method, headers: item.headers, credentials: item.credentials ? 'include' : 'omit'}}); + allowed = response.status === 200 && await response.text() === 'ok'; + }} catch (error) {{ if (!(error instanceof TypeError)) throw error; }} + }} else {{ + const xhr = new XMLHttpRequest(); + const done = new Promise(resolve => xhr.onloadend = resolve); + xhr.open(item.method, url, api !== 'sync-xhr'); + xhr.withCredentials = item.credentials; + for (const [name, value] of item.headers) xhr.setRequestHeader(name, value); + if (item.upload) xhr.upload.onprogress = () => {{}}; + try {{ + xhr.send('payload'); + if (api !== 'sync-xhr') await done; + allowed = xhr.status === 200 && xhr.responseText === 'ok'; + }} catch (error) {{ if (api !== 'sync-xhr' || error.name !== 'NetworkError') throw error; }} + }} + results.push([item.label, allowed]); + }} + return JSON.stringify(results); + }})() + "#, + serde_json::to_string(&inputs).unwrap() + ); + let script = if worker { + let worker_script = format!( + "Promise.resolve().then(() => {probe}).then(value => {{ postMessage(value); close(); }}, error => {{ postMessage(String(error.stack || error)); close(); }});" + ); + format!( + r#" + globalThis.preflightResult = 'pending'; + const worker = new Worker(URL.createObjectURL(new Blob([{}], {{type: 'text/javascript'}}))); + worker.onmessage = event => {{ preflightResult = event.data; }}; + worker.onerror = event => {{ preflightResult = event.message; event.preventDefault(); }}; + "#, + serde_json::to_string(&worker_script).unwrap() + ) + } else { + format!( + "globalThis.preflightResult = 'pending'; Promise.resolve().then(() => {probe}).then(value => {{ preflightResult = value; }}, error => {{ preflightResult = String(error.stack || error); }});" + ) + }; + vm.eval(&script).unwrap(); + tokio::time::timeout(std::time::Duration::from_secs(10), async { + while vm.eval("preflightResult === 'pending'").unwrap() == "true" { + wait_for_one_selected_page_task_executor_test_turn(&mut vm, &loader) + .await + .unwrap(); + } + }) + .await + .expect("preflight permission matrix should finish"); + stop_tx.send(()).unwrap(); + let observed = server.await.unwrap(); + let expected = cases + .iter() + .map(|case| (case.label, case.allowed)) + .collect::>(); + assert_eq!( + vm.eval("preflightResult").unwrap(), + serde_json::to_string(&expected).unwrap(), + "worker={worker}, api={api}" + ); + let expected_requests = cases + .iter() + .enumerate() + .flat_map(|(index, case)| { + std::iter::once((index, "OPTIONS".to_owned())) + .chain(case.allowed.then(|| (index, case.method.to_owned()))) + }) + .collect::>(); + assert_eq!( + observed, expected_requests, + "only approved requests may reach transport: worker={worker}, api={api}" + ); + } +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn cors_preflight_permissions_reach_window_fetch_and_xhr() { + check_preflight_permissions(false).await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn cors_preflight_permissions_reach_worker_fetch_and_xhr() { + check_preflight_permissions(true).await; +} diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/mod.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/mod.rs index 668657df0e..6700298a45 100644 --- a/moli-renderer-v8/src/script_vm/tests/dom_xhr/mod.rs +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/mod.rs @@ -1,6 +1,7 @@ use super::*; mod computed_style; +mod cors_preflight; mod cssom; mod dom; mod file_input;