cef5dec89197db9b2ffff6625b9bd03bb471ffea  Resolve UNSAFE_TODO in FileSystemAccessIncognitoFileDelegate::Read

The bug at a glance

Medium because the pre-fix memcpy copied bytes_to_read (the caller-requested length) rather than bytes_read (the length the file actually delivered), and the surrounding CHECKs only bound bytes_read <= bytes_to_read and buffer->size() <= bytes_to_read. When a short read occurs (bytes_read < bytes_to_read), the destination span ‘data’ is filled with (bytes_to_read - bytes_read) bytes past what the source buffer legitimately populated, exposing stale/uninitialized backing-store contents to the renderer that requested the read. This is a genuine information-disclosure primitive within the incognito FileSystemAccess file emulation. It stays watch-tier and not patch-now because: (1) the copy source ‘buffer’ is a mojo BigBuffer sized to bytes_to_read, so the read stays inside the allocation and is not a heap-OOB that crashes or corrupts adjacent objects; (2) the disclosed bytes are only whatever the same renderer’s incognito in-memory file backing already held, not cross-origin or cross-process secrets; (3) triggering a partial read reliably and observing the leaked tail is non-trivial. Realistically this is a latent-correctness/hardening fix with a plausible info-leak flavor.

Defenders should read this as a quiet correctness fix to Blink’s incognito FileSystemAccess emulation where a Read() that returns fewer bytes than requested previously copied the full requested length back to the caller. The tail beyond the actually-read region could carry stale bytes from the emulated file’s backing buffer, a small same-context information-disclosure surface rather than a memory-corruption bug. It is a good reminder to audit every remaining UNSAFE_TODO(memcpy(…)) in Blink for the classic ‘copied requested length instead of returned length’ mistake, but on its own it does not warrant emergency patching.

Root cause

In FileSystemAccessIncognitoFileDelegate::Read, the incognito file system does not touch disk; reads are serviced from an in-memory mojo buffer. The delegate asks the browser-side incognito file host to read up to bytes_to_read bytes into ‘buffer’ and receives back the actual bytes_read count. The destination is a WTF byte span ‘data’ handed in by the caller. The pre-fix code performed UNSAFE_TODO(memcpy(data.data(), buffer->data(), bytes_to_read)) after two CHECKs: CHECK_LE(bytes_read, bytes_to_read) and CHECK_LE(buffer->size(), static_cast<uint64_t>(bytes_to_read)).

The defect is the copy length. On a short read the file host populates only the first bytes_read entries of ‘buffer’; the remaining (bytes_to_read - bytes_read) bytes are whatever the buffer allocation already contained. Copying bytes_to_read therefore writes that unpopulated/stale tail into ‘data’, which is subsequently returned to the requesting renderer script. Because the CHECKs guarantee the source allocation is at least as large as the copy, this does not run off the end of ‘buffer’ (so no hard crash / adjacent-object corruption), but it does over-report data content: the caller sees more ‘valid’ bytes than were actually read, and those extra bytes are internal buffer residue. The fix copies exactly bytes_read: data.copy_prefix_from(base::span(*buffer).first(base::checked_cast<size_t>(bytes_read))). This aligns the returned content length with the true read length and, as a bonus, eliminates the raw memcpy in favor of a bounds-checked span copy (copy_prefix_from CHECKs the destination is large enough).

Key code

Copy the bytes actually read, not the bytes requested (file_system_access_incognito_file_delegate.cc)

-    UNSAFE_TODO(memcpy(data.data(), buffer->data(), bytes_to_read));
+    data.copy_prefix_from(
+        base::span(*buffer).first(base::checked_cast<size_t>(bytes_read)));

Patch walkthrough

• third_party/blink/renderer/modules/file_system_access/file_system_access_incognito_file_delegate.cc — Inside the successful-read branch (guarded by CHECK_LE(bytes_read, bytes_to_read) and CHECK_LE(buffer->size(), bytes_to_read)), the memcpy of bytes_to_read is replaced by a span copy of exactly bytes_read. base::span(*buffer).first(checked_cast<size_t>(bytes_read)) forms a source view of the populated prefix, and data.copy_prefix_from(…) copies it into the caller’s span while asserting the destination is large enough. This changes semantics: the returned buffer now reflects only the bytes the read produced.
• third_party/blink/renderer/modules/file_system_access/file_system_access_incognito_file_delegate.cc — The include of base/compiler_specific.h is removed because UNSAFE_TODO (which lives there) is no longer used. This is the mechanical signal that the last unsafe raw-buffer operation in this file has been retired.

Background

UNSAFE_TODO — A Chromium macro marking a raw pointer/length operation (here memcpy) that has not yet been migrated to spanification. It suppresses the -Wunsafe-buffer-usage warning and flags the site for later review. Resolving one means proving and enforcing the bounds, which is exactly where latent length bugs surface.

FileSystemAccess incognito delegate — In incognito/OTR profiles the File System Access API cannot persist to disk, so file contents are emulated in an in-memory backing store owned by the browser process and accessed over mojo. This delegate is the renderer-side adapter that services Read/Write against that emulated store.

bytes_read vs bytes_to_read — bytes_to_read is the caller’s requested length; bytes_read is what the read actually delivered. Short reads (bytes_read < bytes_to_read) are normal near EOF or on partial fills. Using the request length as a copy length is the canonical short-read info-leak bug.

copy_prefix_from / span::first — base::span helpers. first(n) narrows a span to its first n elements (CHECKing n <= size); copy_prefix_from copies a source span into the prefix of the destination, CHECKing the destination is at least as large. Together they make the copy length explicit and bounds-checked.

Vulnerability window

1. Introduction — The incognito delegate historically used a raw memcpy of bytes_to_read, marked UNSAFE_TODO during the spanification effort. The length mismatch predates this commit.
2. Discovery — Surfaced while resolving the UNSAFE_TODO (crbug 451652367). The author notes copying bytes_read ‘seems more in line with expected behavior’, i.e. the mismatch was noticed during the safety migration, not from an external report.
3. Fix — 2026-01-20, main@{#1571962}, authored by Fredrik Soderquist (Opera), reviewed by Rakina Zata Amni. Landed as a spanification/correctness change, not tagged as a security release.

Proof of concept

No test or PoC shipped with the commit. To reconstruct the trigger: in an incognito context, obtain a FileSystemAccess file handle backed by the in-memory delegate, then issue a read whose requested length exceeds the remaining available bytes (e.g. request N bytes when only M<N are present, or read past logical EOF into a larger allocated buffer). Pre-fix, the returned buffer would contain M valid bytes followed by (N-M) bytes of whatever the reused mojo buffer allocation held. Observing meaningful residue would require the allocation to have previously held data of interest, which is not guaranteed. No working exploit is demonstrated and none should be assumed.

Exploitation

1. Precondition — Attacker script must run in an incognito/OTR profile with File System Access granted, and must be able to force a short read (bytes_read < bytes_to_read).
2. Primitive — Same-context information disclosure: reads back up to (bytes_to_read - bytes_read) bytes of uninitialized/stale buffer memory. Bounded strictly inside the source allocation by the CHECKs, so no OOB crash or corruption.
3. Blockers — Leaked content is limited to the same renderer’s own incognito buffer residue, not cross-origin/cross-process data. Buffer contents are non-deterministic; reliably staging interesting residue and exfiltrating the tail is impractical. This is why it is watch-tier, not exploitable-grade.

Detection & hunting

For defenders and detection engineers:

• Source audit — Grep Blink and //storage for memcpy/copy calls that use a requested/capacity length (bytes_to_read, buffer size) as the copy count instead of the returned/actual length. Remaining UNSAFE_TODO(memcpy(…)) sites are the highest-yield candidates.
• Runtime — Not practically detectable at runtime; a short-read over-copy produces no crash and no distinctive signal, so do not expect crash reports or DumpWithoutCrashing entries. This fix removes a silent content bug, not a crasher.

Audit directions

• Blink FileSystemAccess — Review the write path and any other Read/Write helpers in the incognito and on-disk delegates for the same request-length vs actual-length pattern, and confirm callers of Read do not trust bytes beyond the returned count.
• Spanification backlog — Treat every UNSAFE_TODO(memcpy(dst, src, LEN)) where LEN is a requested/max length as a candidate short-read leak; prioritize sites where src is a reused/pooled buffer (mojo BigBuffer, IPC scratch) whose tail can carry prior data.