document.write() inserts into the document's input stream. There is one
stream and one insertion point per document, and the tokenizer carries
its state across calls, so a construct may be split anywhere, even in
the middle of a tag name.
Obscura parsed every call as a standalone fragment. Anything spanning
two calls was lost and stayed in the body as text. The SAP UI5
cachebuster writes exactly that way, one call for the tag, one per
attribute, then '>' and the closing tag, so none of its bootstrap parts
were ever requested.
Obscura parses a document in one pass, so there is no live tokenizer to
join. This keeps one parser per document instead, which is what gives
the calls a shared tokenizer state. It parses into its own tree and
mirrors that into the document as it grows. Only <script> and <template>
wait until they are complete, a script because inserting it runs it, a
template because its children live in a separate contents document that
a child walk never reaches.
Which nodes are finished comes from TreeBuilder::trace_handles, not from
TreeSink::pop. pop looks like the signal, and its doc says so, but of
the three paths that pop the stack of open elements only one reports to
the sink; an end tag usually takes pop_until, which does not.
Written nodes also went to the end of the body rather than to the
insertion point, so a script in the <head> inserted behind everything
the parser had already seen. bootstrap.js already tracks the running
script in __currentScriptNid, and the point advances with every node
placed, across calls too.
The host creates the nodes without attaching them and returns where each
belongs. bootstrap.js attaches them with appendChild and insertBefore so
the insertion steps run.
The mirroring walk starts at the last child and stops at the first node
already handed over, because the parser only appends. Reading the whole
child list per call made the cost grow with the stream: 852 ms against
370 ms over 5000 calls.
Interleaved against d9ef40b, same release build, fifteen runs per
revision, 5000 operations per case, medians:
insertBefore 168 ms -> 168 ms 0.0%
replaceChild 299 ms -> 316 ms +5.7%
insertBefore w/ observer 163 ms -> 182 ms +11.7%
document.write 553 ms -> 557 ms +0.7%
The observer case is the only one that adds work. It is the alignment
with appendChild, which always reported: on this branch insertBefore
with an active observer costs 165 ms, appendChild 160 ms.
document.open() drops the stream, since a fresh parse begins.
appendChild and removeChild report a childList mutation, and appendChild
loads a written stylesheet. insertBefore and replaceChild did neither,
so an observer on a parent saw a node appear only when it went to the
end, and a <link> placed anywhere else never loaded.
before(), after() and replaceWith() all route through insertBefore, so
they were affected too. The new test walks those plus replaceChild on
connected elements, which is the check AGENTS.md asks for after touching
a mutation method. Argument order was correct everywhere, only the
reporting was missing.
descendants() already caps its walk at nodes.len() as defense-in-depth
against a corrupted/cyclic node graph, but children() (sibling chain) and
ancestors() (parent chain) looped over next_sibling / parent pointers
with no bound. A single corrupted pointer would hang these walkers — and
every caller — forever, while descendants() would recover.
The append_child / insert_before guards prevent such cycles through the
public API, so this is hardening, not a reachable bug. Mirror the
descendants() bound in both loops: stop once the collected count exceeds
nodes.len().
Adds tests that forge a sibling cycle and a parent cycle by writing the
node arena directly and assert each walk stays bounded instead of
hanging.
Closes#582
crypto.subtle.deriveBits/deriveKey for PBKDF2 passed the iteration count
and output length from page JS straight into op_subtle_pbkdf2 with no
bound. Because the JS runtime is single-threaded and shared across the
CDP connection, a page calling deriveBits with iterations=4294967295
pins the V8 isolate for hours and blocks every other command; a huge
requested length forces an unbounded vec![0u8; length] allocation.
Split the derivation into a testable pbkdf2_derive helper that rejects
iteration counts above 10_000_000 and output lengths above 1 MiB — both
far above any legitimate use (OWASP recommends ~600k iterations; derived
keys are tens of bytes) — with an OperationError before doing the work.
Closes#580
DOM.setFileInputFiles read client-supplied local file paths from disk
and handed their bytes to page JS with no access control. Any client
that can reach the CDP port (default localhost, but Docker images bind
0.0.0.0) could thus read any file the process can read — e.g.
setFileInputFiles with files=["/etc/passwd"].
Page.navigate to file:// already guards this exact threat behind
context.allow_file_access (off by default; opt in with
`obscura serve --allow-file-access`). Apply the same gate here, refusing
with a matching error before any std::fs::read when the flag is off.
Adds a test proving the handler refuses to read an existing file when
allow_file_access is off.
Closes#579
Runtime.removeBinding interpolated the client-supplied binding name
directly into `delete globalThis['{name}']` with no validation, so a
CDP client could break out of the string literal and execute arbitrary
JS in the page context — e.g. name = "x'] , (evil()) , globalThis['y".
Runtime.addBinding already guarded against this by requiring the name to
be a plain identifier. Extract that check into a shared
`is_valid_binding_name` helper and gate removeBinding behind it too; a
non-identifier name is silently ignored (there is nothing to delete).
Adds an end-to-end test that drives removeBinding against a live page
with a comma-expression payload and asserts the injected assignment
never runs.
Closes#578
Two lines in crates/obscura/src/page.rs call methods directly on
self.inner, which is now a RefCell<Page>. So main does not build:
error[E0599]: no method named `frame_urls` found for struct `RefCell<T>`
error[E0599]: no method named `evaluate_in_frame` found for struct `RefCell<T>`
It is a conflict between two changes that were each correct on their
own. One introduced the two wrappers, another turned Page.inner into
a RefCell. Both ran green against the main of their time. The merge
does not see it, because no textual conflict arises.
The neighbor shows how it was meant: evaluate directly above already
borrows. The two frame wrappers were overlooked during the rework.
frame_urls reads, so borrow(). evaluate_in_frame takes &mut self,
so borrow_mut().
No new tests. crates/obscura/tests/child_frame_scripts.rs already
uses both methods in 14 places, and it just would not
build as
long as the library does not build.
HTTP-redirect fetch returns a network error as soon as a request's
redirect count reaches 20, and only increments the counter
afterwards. The twentieth hop must still succeed and the
twenty-first must fail. FETCH_REDIRECT_LIMIT was 10, so fetch() and
XMLHttpRequest gave up after half the redirects the standard allows.
https://fetch.spec.whatwg.org/#http-redirect-fetch
WPT covers both ends of the pair in
fetch/api/redirect/redirect-count.any.js: twenty hops arrive,
twenty-one are rejected. New tests against a local redirect chain
cover both.
The comment claimed the value matched the reqwest default. It does
not apply here: the redirects are followed by hand in ops.rs, one
hop per loop iteration, so that each hop is checked against the
SSRF rules again.
SGavrl