fix(standalone): bound the CF_UNICODETEXT scan by the clipboard block's own size - #427
Conversation
Deploying mouseterm with
|
| Latest commit: |
b50c587
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c4053c86.mouseterm.pages.dev |
| Branch Preview URL: | https://fix-clipboard-win-bounded-te.mouseterm.pages.dev |
dormouse-bot
left a comment
There was a problem hiding this comment.
Reviewed; no correctness findings. The bound is right — size / 2 can never name a unit past the allocation, the terminator only shortens, and the caller (read_clipboard_text in standalone/src-tauri/src/lib.rs) does read_text().unwrap_or_default(), so the new Some("") on a zero-size block is indistinguishable from the old path.
Closing the "I could not compile this" caveat. I built the module for the Windows target here, and it's clean:
cargo check --target x86_64-pc-windows-msvcoverclipboard_win.rsin a scratch crate carrying the samewindows = "0.62"feature set asstandalone/src-tauri/Cargo.toml— compiles with no errors and no warnings. Re-run with--profile test --all-targets(rustc invoked with--test, confirmed in-voutput) so the#[cfg(test)]module typechecks too.- All 7 tests in that module pass — the three new
utf16_up_to_nulcases plus the four pre-existingbmp_file_headerones — extracted with their dependencies and run viarustc --test.
That's a real build of the surrounding module, not just the helper, so the merge doesn't rest on release.yml being the first thing to compile it. It still isn't a check on this PR, which is what #428 fixes.
How the Windows-target check was run
cargo check --target x86_64-pc-windows-msvc on standalone/src-tauri itself fails before reaching this crate — a transitive cc-driven build script wants lib.exe. Isolating the module sidesteps that:
rustup target add x86_64-pc-windows-msvc
# scratch crate: only the `windows` 0.62 dep with Cargo.toml's five features
cp standalone/src-tauri/src/clipboard_win.rs /tmp/cw/src/lib.rs
cargo check --target x86_64-pc-windows-msvc # clean
cargo check --profile test --all-targets --target x86_64-pc-windows-msvc # cleanThe module is self-contained (std + windows only), so nothing had to be stubbed. For the run, the pure items (utf16_up_to_nul, bmp_file_header, BI_BITFIELDS, the tests module) were extracted verbatim and compiled natively with rustc --test --edition 2021: 7 passed; 0 failed.
read_textscanned the lockedCF_UNICODETEXTblock for a NUL terminator without bounding the scan byGlobalSize, which it took as_sizeand discarded. The terminator is the producing process's promise, not something this one can check, so a block written without one sends the loop past the end of the allocation — an out-of-bounds read inunsafecode whose payload is pasted straight into the user's terminal. The scan is now bounded by the block's own size, and the terminator only shortens the result.Surfaced by the nightly code-quality survey (
standalone/src-tauri/src/clipboard_win.rswas in today's rotation).The read, before and after
Before, the size was named and dropped, and the loop had nothing to stop it:
After, the slice is cut to
size / 2first — so it can never name memory past the allocation, and an odd byte count drops the trailing half unit rather than reading one byte beyond — and the terminator search runs inside it. The search itself moved into a plain&[u16]helper (utf16_up_to_nul) so the termination rule is unit-testable without a Win32 clipboard.Its two sibling readers were already bounded and are unchanged:
read_file_pathssizes each buffer fromDragQueryFileW, andread_dib_bytescopies exactlysizebytes.Verification. Three tests pin the helper: a terminated block with slack after the NUL (the well-formed case —
GlobalSizerounds the allocation up, so the slack must not reach the caller), an unterminated block (yields exactly its own bytes), and the empty/bare-NUL block.Two caveats on how far that verification reaches, both worth stating plainly:
HGLOBALwhose contents run to the end of the allocation, which is a Win32 clipboard, not a#[test]. The tests pin the contract the fix introduces — bounded scan, terminator-shortens-only — rather than the crash they prevent.clipboard_winis behind#[cfg(windows)], and CI's only Rust step iscargo checkonubuntu-latest, so this file is not built by any check on this PR. The helper's body and its three tests were compiled and run standalone withrustc --test(3 passed); the surrounding module was not. That gap is a separate finding — see the companion PR adding a Windows/macOScargo checkjob — and until it merges, the first build of this file happens inrelease.ymlon av*tag.