fix(cdp): gate Target.createTarget file:// like Page.navigate
GHSA-q55h-vfv9-qcr5 patched Page.navigate to refuse file:// URLs
unless --allow-file-access was set, but Target.createTarget accepted
the same scheme and called page.navigate(url) directly, bypassing
the policy. A CDP client could call Target.createTarget {url: file://...}
then Runtime.evaluate to read document.body.textContent and recover
arbitrary local files.
Apply the same gate at the Target.createTarget entrypoint. Pull
the file-scheme detector into a shared util module so both domains
stay in lock-step instead of one drifting from the other.
Verified blocked vs the unpatched build (pr69):
- Patched: Target.createTarget {url: file:///tmp/secret} errors
"Target.createTarget to file:// is disabled. ..." before any
navigation runs.
- Unpatched: same call returns the file contents via
Runtime.evaluate {expression: "document.body.textContent"}.
This commit is contained in:
1 parent
935cb5c0da
commit
3158a1fd31
4 files changed
+76
-8
No files matched your search
@@ -3,14 +3,7 @@ use serde_json::{json, Value};
|
|||||||
|
|
||||||
use crate::dispatch::CdpContext;
|
use crate::dispatch::CdpContext;
|
||||||
use crate::types::CdpEvent;
|
use crate::types::CdpEvent;
|
||||||
|
use crate::util::url_is_file_scheme;
|
||||||
fn url_is_file_scheme(raw: &str) -> bool {
|
|
||||||
url::Url::parse(raw)
|
|
||||||
.map(|u| u.scheme().eq_ignore_ascii_case("file"))
|
|
||||||
.unwrap_or_else(|_| {
|
|
||||||
raw.trim_start().to_ascii_lowercase().starts_with("file:")
|
|
||||||
})
|
|
||||||
}
|
|
||||||
|
|
||||||
pub async fn handle(
|
pub async fn handle(
|
||||||
method: &str,
|
method: &str,
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ use serde_json::{json, Value};
|
|||||||
|
|
||||||
use crate::dispatch::CdpContext;
|
use crate::dispatch::CdpContext;
|
||||||
use crate::types::CdpEvent;
|
use crate::types::CdpEvent;
|
||||||
|
use crate::util::url_is_file_scheme;
|
||||||
|
|
||||||
pub async fn handle(method: &str, params: &Value, ctx: &mut CdpContext) -> Result<Value, String> {
|
pub async fn handle(method: &str, params: &Value, ctx: &mut CdpContext) -> Result<Value, String> {
|
||||||
match method {
|
match method {
|
||||||
@@ -55,6 +56,17 @@ pub async fn handle(method: &str, params: &Value, ctx: &mut CdpContext) -> Resul
|
|||||||
}
|
}
|
||||||
"createTarget" => {
|
"createTarget" => {
|
||||||
let url = params.get("url").and_then(|v| v.as_str()).unwrap_or("about:blank");
|
let url = params.get("url").and_then(|v| v.as_str()).unwrap_or("about:blank");
|
||||||
|
|
||||||
|
// Same gate as Page.navigate (GHSA-q55h-vfv9-qcr5). Without this,
|
||||||
|
// a CDP client can call Target.createTarget {url:"file:///etc/passwd"}
|
||||||
|
// and then Runtime.evaluate the body off the created target,
|
||||||
|
// bypassing the page-domain check entirely.
|
||||||
|
if url_is_file_scheme(url) && !ctx.default_context.allow_file_access {
|
||||||
|
return Err(
|
||||||
|
"Target.createTarget to file:// is disabled. Restart with `obscura serve --allow-file-access` to enable.".to_string()
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
let page_id = ctx.create_page();
|
let page_id = ctx.create_page();
|
||||||
let session_id = format!("{}-session", page_id);
|
let session_id = format!("{}-session", page_id);
|
||||||
|
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ pub mod server;
|
|||||||
pub mod dispatch;
|
pub mod dispatch;
|
||||||
pub mod types;
|
pub mod types;
|
||||||
pub mod domains;
|
pub mod domains;
|
||||||
|
pub(crate) mod util;
|
||||||
|
|
||||||
pub use server::{
|
pub use server::{
|
||||||
start, start_with_full_options, start_with_host, start_with_host_and_security,
|
start, start_with_full_options, start_with_host, start_with_host_and_security,
|
||||||
|
|||||||
@@ -0,0 +1,62 @@
|
|||||||
|
/// Helpers shared across CDP domain handlers.
|
||||||
|
///
|
||||||
|
/// The file-scheme detector here is reused by every CDP entrypoint that can
|
||||||
|
/// trigger a navigation, so we don't end up with one domain enforcing the
|
||||||
|
/// `--allow-file-access` gate and another silently letting `file://` through
|
||||||
|
/// (see GHSA-q55h-vfv9-qcr5 and its incomplete-fix variant in
|
||||||
|
/// `Target.createTarget`).
|
||||||
|
|
||||||
|
/// Returns true when `raw` parses as a `file:`-scheme URL, or syntactically
|
||||||
|
/// starts with `file:` after a possible leading-whitespace strip. Matching is
|
||||||
|
/// case-insensitive on the scheme so neither `FILE://` nor `File://` slips
|
||||||
|
/// past callers that gate on `file://`.
|
||||||
|
pub(crate) fn url_is_file_scheme(raw: &str) -> bool {
|
||||||
|
url::Url::parse(raw)
|
||||||
|
.map(|u| u.scheme().eq_ignore_ascii_case("file"))
|
||||||
|
.unwrap_or_else(|_| {
|
||||||
|
raw.trim_start().to_ascii_lowercase().starts_with("file:")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn matches_plain_file_url() {
|
||||||
|
assert!(url_is_file_scheme("file:///etc/passwd"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn matches_case_insensitively() {
|
||||||
|
assert!(url_is_file_scheme("FILE:///etc/passwd"));
|
||||||
|
assert!(url_is_file_scheme("File:///etc/passwd"));
|
||||||
|
assert!(url_is_file_scheme("fIlE:///etc/passwd"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn matches_with_leading_whitespace_fallback() {
|
||||||
|
// url::Url::parse rejects leading whitespace, but the syntactic
|
||||||
|
// fallback still catches ` file:...` so callers can't be tricked
|
||||||
|
// into letting it through.
|
||||||
|
assert!(url_is_file_scheme(" file:///etc/passwd"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rejects_http_https_about_data() {
|
||||||
|
assert!(!url_is_file_scheme("http://example.com"));
|
||||||
|
assert!(!url_is_file_scheme("https://example.com"));
|
||||||
|
assert!(!url_is_file_scheme("about:blank"));
|
||||||
|
assert!(!url_is_file_scheme("data:text/plain,hi"));
|
||||||
|
assert!(!url_is_file_scheme(""));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rejects_lookalikes_that_are_not_file_scheme() {
|
||||||
|
// The URL parser rejects these (no `://`), so the syntactic fallback
|
||||||
|
// kicks in. `file` appearing anywhere except as the leading scheme
|
||||||
|
// must not match.
|
||||||
|
assert!(!url_is_file_scheme("notfile:///x"));
|
||||||
|
assert!(!url_is_file_scheme("http://file/"));
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user