Skip to content

fix(standalone): bound the CF_UNICODETEXT scan by the clipboard block's own size - #427

Merged
nedtwigg merged 1 commit into
mainfrom
fix/clipboard-win-bounded-text-scan
Aug 21, 2026
Merged

fix(standalone): bound the CF_UNICODETEXT scan by the clipboard block's own size#427
nedtwigg merged 1 commit into
mainfrom
fix/clipboard-win-bounded-text-scan

Conversation

@dormouse-bot

Copy link
Copy Markdown
Collaborator

read_text scanned the locked CF_UNICODETEXT block for a NUL terminator without bounding the scan by GlobalSize, which it took as _size and 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 in unsafe code 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.rs was in today's rotation).

The read, before and after

Before, the size was named and dropped, and the loop had nothing to stop it:

with_locked_clipboard(CF_UNICODETEXT.0, |ptr, _size| {
    unsafe {
        let wide = ptr as *const u16;
        let mut len = 0usize;
        while *wide.add(len) != 0 {
            len += 1;
        }
        Some(String::from_utf16_lossy(std::slice::from_raw_parts(wide, len)))
    }
})

After, the slice is cut to size / 2 first — 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_paths sizes each buffer from DragQueryFileW, and read_dib_bytes copies exactly size bytes.

Verification. Three tests pin the helper: a terminated block with slack after the NUL (the well-formed case — GlobalSize rounds 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:

  • The OOB itself has no unit test. Reproducing it needs a real HGLOBAL whose 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.
  • I could not compile this. clipboard_win is behind #[cfg(windows)], and CI's only Rust step is cargo check on ubuntu-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 with rustc --test (3 passed); the surrounding module was not. That gap is a separate finding — see the companion PR adding a Windows/macOS cargo check job — and until it merges, the first build of this file happens in release.yml on a v* tag.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying mouseterm with  Cloudflare Pages  Cloudflare Pages

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

View logs

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-msvc over clipboard_win.rs in a scratch crate carrying the same windows = "0.62" feature set as standalone/src-tauri/Cargo.toml — compiles with no errors and no warnings. Re-run with --profile test --all-targets (rustc invoked with --test, confirmed in -v output) so the #[cfg(test)] module typechecks too.
  • All 7 tests in that module pass — the three new utf16_up_to_nul cases plus the four pre-existing bmp_file_header ones — extracted with their dependencies and run via rustc --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  # clean

The 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.

@nedtwigg
nedtwigg merged commit 1f6ae11 into main Aug 21, 2026
8 checks passed
@nedtwigg
nedtwigg deleted the fix/clipboard-win-bounded-text-scan branch August 21, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants