Validate guest-writable fields in try_pop_buffer_into before allocation

[ludfjig]

try_pop_buffer_into reads two values from guest-writable shared memory

• Size prefix: A malicious guest can set this to near u32::MAX, causing the host to attempt a ~4 GB allocation via vec![0; fb_buffer_size] (DoS).
• Back-pointer: A malicious guest can point this outside the buffer, causing fill(0, ...) to zero unrelated host-managed data within shared memory (e.g., PEB fields, input buffer) and leaving the buffer stack in an inconsistent state.
• Arithmetic overflow/underflow: A back-pointer just below the stack pointer causes underflow in stack_pointer_rel - last_element_offset_rel - 8. A size prefix of 0xFFFF_FFFD causes +4 to wrap around in debug (panic) or release (silent wrap to a small value).

Changes:

• Validate last_element_offset_rel (back-pointer) is within [[8, stack_pointer_rel) before use
• Validate fb_buffer_size does not exceed the actual element slot size before allocation
• Use checked_add(4) on the size prefix instead of bare + 4, returning a descriptive error on u32 overflow
• Add unit tests covering corrupt size prefix, corrupt back-pointer (too small / too large), off-by-one, back-pointer near stack pointer (underflow), and u32 size prefix overflow
• Add integration tests with guest functions that write corrupt output buffers and halt

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[syntactically]

The arithmetic overflows causing unintentional panics issue I bet is something we have in other places. (I stand by some of the arguments I made earlier about panic-ing being reasonable behaviour in some situations---but it's important that we are directly aware of the panic opportunities and clear on whether they make sense---i.e. we should have a sensible reasoned argument why they ought to be impossible, even in the face of attacker-controlled data). It would be interesting to take a look at e.g. enumerating every cfg path from any exported function to panic (on an opt build, so that a bunch of the easy ones are gone) and see how much work it would be to make an argument either way on each path.