Two follow-ups from reviewing #8868's addon load path. Neither is a live vulnerability — both are hardening.
1. entry ∈ files is an implicit invariant, not an asserted one
verify_addon_payload (node_api_host/loader.rs:198) size- and SHA-256-checks every file in addon.files, then returns safe_payload_path(root, &addon.entry). Nothing at the load site asserts that the resolved entry is among the files just verified — so, read in isolation, the file handed to dlopen need not be one that was hashed.
It is safe today only because of a distant invariant in the staging step: native_addon_sidecar.rs::addon_payload_files walks the package directory, and only walked files are both copied and hashed into files. An entry that was never hashed was therefore never staged, so canonicalize fails and the load errors out.
That is genuinely fail-closed, but the guarantee lives in a different crate from the code that depends on it. A future change to the staging walk — an added copy path, a filter tweak — could break it silently, and the load path would not notice.
Suggested: assert at load time that the resolved entry path is present in the verified set, so the load path is self-evidently safe rather than safe-by-construction-elsewhere.
2. TOCTOU between hashing and dlopen
verify_addon_payload reads and hashes the file; open_library then opens it again from disk. Between those two reads the bytes can change.
Exploiting this requires local write access to the staged sidecar directory, which is largely outside the threat model this feature targets — so severity is low. It is still a real window, and worth recording rather than rediscovering.
Options: hold an open file descriptor across verification and load (fdlopen where available), or accept and document it explicitly.
Found while auditing #8868 (landed via #8870). The verify-then-load ordering and the canonical-path traversal check are both correct as written.
Two follow-ups from reviewing #8868's addon load path. Neither is a live vulnerability — both are hardening.
1.
entry ∈ filesis an implicit invariant, not an asserted oneverify_addon_payload(node_api_host/loader.rs:198) size- and SHA-256-checks every file inaddon.files, then returnssafe_payload_path(root, &addon.entry). Nothing at the load site asserts that the resolved entry is among the files just verified — so, read in isolation, the file handed todlopenneed not be one that was hashed.It is safe today only because of a distant invariant in the staging step:
native_addon_sidecar.rs::addon_payload_fileswalks the package directory, and only walked files are both copied and hashed intofiles. An entry that was never hashed was therefore never staged, socanonicalizefails and the load errors out.That is genuinely fail-closed, but the guarantee lives in a different crate from the code that depends on it. A future change to the staging walk — an added copy path, a filter tweak — could break it silently, and the load path would not notice.
Suggested: assert at load time that the resolved entry path is present in the verified set, so the load path is self-evidently safe rather than safe-by-construction-elsewhere.
2. TOCTOU between hashing and
dlopenverify_addon_payloadreads and hashes the file;open_librarythen opens it again from disk. Between those two reads the bytes can change.Exploiting this requires local write access to the staged sidecar directory, which is largely outside the threat model this feature targets — so severity is low. It is still a real window, and worth recording rather than rediscovering.
Options: hold an open file descriptor across verification and load (
fdlopenwhere available), or accept and document it explicitly.Found while auditing #8868 (landed via #8870). The verify-then-load ordering and the canonical-path traversal check are both correct as written.