Skip to content

Why: - #984

Merged
copybara-service[bot] merged 1 commit into
mainfrom
test_984894771
Sep 21, 2026
Merged

Why:#984
copybara-service[bot] merged 1 commit into
mainfrom
test_984894771

Conversation

@copybara-service

@copybara-service copybara-service Bot commented Sep 20, 2026

Copy link
Copy Markdown

Why:

  • Packet bounds checks used additive arithmetic (offset + size_bytes > host_size), which can wrap around on offsets near 4 GiB, allowing out-of-bounds requests to pass validation.
  • Memory access subsequently occurs outside the staging buffer in 64-bit space, causing silent memory corruption or SIGSEGV crashes.
  • In ProcessSocketBufferPush, connection acquisition occurred prior to request validation, holding connection resources unnecessarily when a request was malformed or out-of-bounds.
  • To maintain wire protocol compatibility across running clusters without version bumps or binary layout changes, the wire format is kept unchanged, with strict sender-side validation preventing 32-bit integer truncation.

Code illustration:

// 32-bit addition wraps around:
const uint32_t dst_offset = 0xFFFFF000;  // ~4 GiB
const uint32_t size_bytes = 0x2000;      // 8 KiB
if (dst_offset + size_bytes > host_size) // 0x1000 <= host_size (Passes!)

// 64-bit pointer does NOT wrap -> writes ~4 GiB outside buffer:
uint8_t* dest_ptr = base_host_ptr + dst_offset;

What:

  • Enforce non-overflowing subtraction bounds checks (size_bytes > host_size || offset > host_size - size_bytes) in ProcessPeerRequest, batched push, and PullBuffer.
  • Add sender-side ceiling checks against UINT32_MAX on remote_id and len in ProcessSocketBufferPull and ProcessSocketBufferPush before populating 32-bit wire fields.
  • Move request opcode and bounds validation before BorrowConnection in ProcessSocketBufferPush to avoid holding pooled connections on invalid requests.
  • Retain wire protocol layout and version (ver = 1) to preserve backward/forward compatibility.
  • Add regression tests verifying wrapping offsets are rejected on the wire and destination bounds violations are caught.

Sequence of Events:

  1. Peer sends a packet with an offset near 4 GiB.
  2. Additive arithmetic wraps around, passing buffer bounds validation.
  3. 64-bit pointer calculation writes outside the buffer, corrupting memory or crashing.

@google-cla

google-cla Bot commented Sep 20, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@copybara-service copybara-service Bot changed the title [Raiden/FaultTolerance] Fix [Corruption/BoundsOverflow]: 64-bit and non-overflowing bounds checks on data port Why: Sep 20, 2026
@copybara-service
copybara-service Bot force-pushed the test_984894771 branch 2 times, most recently from 06f4178 to 8f07699 Compare September 21, 2026 02:55
- Packet bounds checks used additive arithmetic (`offset + size_bytes > host_size`), which can wrap around on offsets near 4 GiB, allowing out-of-bounds requests to pass validation.
- Memory access subsequently occurs outside the staging buffer in 64-bit space, causing silent memory corruption or SIGSEGV crashes.
- In `ProcessSocketBufferPush`, connection acquisition occurred prior to request validation, holding connection resources unnecessarily when a request was malformed or out-of-bounds.
- To maintain wire protocol compatibility across running clusters without version bumps or binary layout changes, the wire format is kept unchanged, with strict sender-side validation preventing 32-bit integer truncation.

Code illustration:
```cpp
// 32-bit addition wraps around:
const uint32_t dst_offset = 0xFFFFF000;  // ~4 GiB
const uint32_t size_bytes = 0x2000;      // 8 KiB
if (dst_offset + size_bytes > host_size) // 0x1000 <= host_size (Passes!)

// 64-bit pointer does NOT wrap -> writes ~4 GiB outside buffer:
uint8_t* dest_ptr = base_host_ptr + dst_offset;
```

What:
- Enforce non-overflowing subtraction bounds checks (`size_bytes > host_size || offset > host_size - size_bytes`) in `ProcessPeerRequest`, batched push, and `PullBuffer`.
- Add sender-side ceiling checks against `UINT32_MAX` on `remote_id` and `len` in `ProcessSocketBufferPull` and `ProcessSocketBufferPush` before populating 32-bit wire fields.
- Move request opcode and bounds validation before `BorrowConnection` in `ProcessSocketBufferPush` to avoid holding pooled connections on invalid requests.
- Retain wire protocol layout and version (`ver = 1`) to preserve backward/forward compatibility.
- Add regression tests verifying wrapping offsets are rejected on the wire and destination bounds violations are caught.

Sequence of Events:
1. Peer sends a packet with an offset near 4 GiB.
2. Additive arithmetic wraps around, passing buffer bounds validation.
3. 64-bit pointer calculation writes outside the buffer, corrupting memory or crashing.

PiperOrigin-RevId: 984971986
@copybara-service
copybara-service Bot merged commit 66cda32 into main Sep 21, 2026
1 of 2 checks passed
@copybara-service
copybara-service Bot deleted the test_984894771 branch September 21, 2026 03:03
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.

0 participants