Repository navigation
[XLA] Validate buffer offset and size in BufferAllocation::AddAssignment - #46105
Closed
prasanna8585 wants to merge 2 commits into
Closed
prasanna8585 wants to merge 2 commits into
prasanna8585 wants to merge 2 commits into
Conversation
|
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. |
prasanna8585
force-pushed
the
fix/buffer-assignment-from-proto-negative-offset
branch
from
July 23, 2026 05:52
b15bddf to
e060784
Compare
akuegel
reviewed
Jul 23, 2026
…roto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR openxla#44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior.
prasanna8585
force-pushed
the
fix/buffer-assignment-from-proto-negative-offset
branch
from
July 23, 2026 09:19
e060784 to
6fab2ec
Compare
akuegel
approved these changes
Jul 23, 2026
beckerhe
reviewed
Jul 24, 2026
copybara-service Bot
pushed a commit
that referenced
this pull request
Jul 24, 2026
…ment::FromProto Imported from GitHub PR #46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ec by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 6fab2ec PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Jul 24, 2026
…ment::FromProto Imported from GitHub PR openxla/xla#46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ecd47988e30cfe195becfaecdb641479e8c by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 6fab2ecd47988e30cfe195becfaecdb641479e8c PiperOrigin-RevId: 952638612
…nment Per review feedback from @beckerhe: move the negative offset/size validation from a precondition check in BufferAssignment::FromProto into BufferAllocation::AddAssignment itself, converting the existing CHECK_LE statements to TF_RET_CHECK. This protects every caller of AddAssignment (there are ~9 call sites across buffer_assignment.cc), not just the FromProto deserialization path, and returns an absl::Status error instead of crashing the process when the precondition is violated. Also adds explicit offset >= 0 / size >= 0 checks: converting CHECK_LE(offset, size_) to TF_RET_CHECK alone would not catch a negative offset, since offset <= size_ is still true for any negative offset and non-negative allocation size. All 9 existing call sites already propagate the returned Status via RETURN_IF_ERROR, so no call-site changes were needed beyond removing the now-redundant check in FromProto.
copybara-service Bot
pushed a commit
that referenced
this pull request
Jul 27, 2026
…ment::FromProto Imported from GitHub PR #46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ec by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 6fab2ec PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Jul 27, 2026
…ment::FromProto Imported from GitHub PR openxla/xla#46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ecd47988e30cfe195becfaecdb641479e8c by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 6fab2ecd47988e30cfe195becfaecdb641479e8c PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
that referenced
this pull request
Jul 27, 2026
…ment::FromProto Imported from GitHub PR #46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ec by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 6fab2ec PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Jul 27, 2026
…ment::FromProto Imported from GitHub PR openxla/xla#46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ecd47988e30cfe195becfaecdb641479e8c by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 6fab2ecd47988e30cfe195becfaecdb641479e8c PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
that referenced
this pull request
Jul 27, 2026
…ment::FromProto Imported from GitHub PR #46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ec by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 6fab2ec PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Jul 27, 2026
…ment::FromProto Imported from GitHub PR openxla/xla#46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ecd47988e30cfe195becfaecdb641479e8c by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 6fab2ecd47988e30cfe195becfaecdb641479e8c PiperOrigin-RevId: 952638612
akuegel
approved these changes
Jul 27, 2026
copybara-service Bot
pushed a commit
that referenced
this pull request
Jul 27, 2026
…:AddAssignment Imported from GitHub PR #46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ec by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. -- 5fdce18 by prasanna8585 <[email protected]>: Address review: move fix from FromProto to BufferAllocation::AddAssignment Per review feedback from @beckerhe: move the negative offset/size validation from a precondition check in BufferAssignment::FromProto into BufferAllocation::AddAssignment itself, converting the existing CHECK_LE statements to TF_RET_CHECK. This protects every caller of AddAssignment (there are ~9 call sites across buffer_assignment.cc), not just the FromProto deserialization path, and returns an absl::Status error instead of crashing the process when the precondition is violated. Also adds explicit offset >= 0 / size >= 0 checks: converting CHECK_LE(offset, size_) to TF_RET_CHECK alone would not catch a negative offset, since offset <= size_ is still true for any negative offset and non-negative allocation size. All 9 existing call sites already propagate the returned Status via RETURN_IF_ERROR, so no call-site changes were needed beyond removing the now-redundant check in FromProto. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 5fdce18 PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Jul 27, 2026
…:AddAssignment Imported from GitHub PR openxla/xla#46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ecd47988e30cfe195becfaecdb641479e8c by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. -- 5fdce18c4bdf2e1e53731611bc08938cf3669a05 by prasanna8585 <[email protected]>: Address review: move fix from FromProto to BufferAllocation::AddAssignment Per review feedback from @beckerhe: move the negative offset/size validation from a precondition check in BufferAssignment::FromProto into BufferAllocation::AddAssignment itself, converting the existing CHECK_LE statements to TF_RET_CHECK. This protects every caller of AddAssignment (there are ~9 call sites across buffer_assignment.cc), not just the FromProto deserialization path, and returns an absl::Status error instead of crashing the process when the precondition is violated. Also adds explicit offset >= 0 / size >= 0 checks: converting CHECK_LE(offset, size_) to TF_RET_CHECK alone would not catch a negative offset, since offset <= size_ is still true for any negative offset and non-negative allocation size. All 9 existing call sites already propagate the returned Status via RETURN_IF_ERROR, so no call-site changes were needed beyond removing the now-redundant check in FromProto. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 5fdce18c4bdf2e1e53731611bc08938cf3669a05 PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
that referenced
this pull request
Jul 27, 2026
…:AddAssignment Imported from GitHub PR #46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ec by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. -- 5fdce18 by prasanna8585 <[email protected]>: Address review: move fix from FromProto to BufferAllocation::AddAssignment Per review feedback from @beckerhe: move the negative offset/size validation from a precondition check in BufferAssignment::FromProto into BufferAllocation::AddAssignment itself, converting the existing CHECK_LE statements to TF_RET_CHECK. This protects every caller of AddAssignment (there are ~9 call sites across buffer_assignment.cc), not just the FromProto deserialization path, and returns an absl::Status error instead of crashing the process when the precondition is violated. Also adds explicit offset >= 0 / size >= 0 checks: converting CHECK_LE(offset, size_) to TF_RET_CHECK alone would not catch a negative offset, since offset <= size_ is still true for any negative offset and non-negative allocation size. All 9 existing call sites already propagate the returned Status via RETURN_IF_ERROR, so no call-site changes were needed beyond removing the now-redundant check in FromProto. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 5fdce18 PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Jul 27, 2026
…:AddAssignment Imported from GitHub PR openxla/xla#46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ecd47988e30cfe195becfaecdb641479e8c by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. -- 5fdce18c4bdf2e1e53731611bc08938cf3669a05 by prasanna8585 <[email protected]>: Address review: move fix from FromProto to BufferAllocation::AddAssignment Per review feedback from @beckerhe: move the negative offset/size validation from a precondition check in BufferAssignment::FromProto into BufferAllocation::AddAssignment itself, converting the existing CHECK_LE statements to TF_RET_CHECK. This protects every caller of AddAssignment (there are ~9 call sites across buffer_assignment.cc), not just the FromProto deserialization path, and returns an absl::Status error instead of crashing the process when the precondition is violated. Also adds explicit offset >= 0 / size >= 0 checks: converting CHECK_LE(offset, size_) to TF_RET_CHECK alone would not catch a negative offset, since offset <= size_ is still true for any negative offset and non-negative allocation size. All 9 existing call sites already propagate the returned Status via RETURN_IF_ERROR, so no call-site changes were needed beyond removing the now-redundant check in FromProto. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 5fdce18c4bdf2e1e53731611bc08938cf3669a05 PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
that referenced
this pull request
Jul 27, 2026
…:AddAssignment Imported from GitHub PR #46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ec by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. -- 5fdce18 by prasanna8585 <[email protected]>: Address review: move fix from FromProto to BufferAllocation::AddAssignment Per review feedback from @beckerhe: move the negative offset/size validation from a precondition check in BufferAssignment::FromProto into BufferAllocation::AddAssignment itself, converting the existing CHECK_LE statements to TF_RET_CHECK. This protects every caller of AddAssignment (there are ~9 call sites across buffer_assignment.cc), not just the FromProto deserialization path, and returns an absl::Status error instead of crashing the process when the precondition is violated. Also adds explicit offset >= 0 / size >= 0 checks: converting CHECK_LE(offset, size_) to TF_RET_CHECK alone would not catch a negative offset, since offset <= size_ is still true for any negative offset and non-negative allocation size. All 9 existing call sites already propagate the returned Status via RETURN_IF_ERROR, so no call-site changes were needed beyond removing the now-redundant check in FromProto. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 5fdce18 PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Jul 27, 2026
…:AddAssignment Imported from GitHub PR openxla/xla#46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ecd47988e30cfe195becfaecdb641479e8c by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. -- 5fdce18c4bdf2e1e53731611bc08938cf3669a05 by prasanna8585 <[email protected]>: Address review: move fix from FromProto to BufferAllocation::AddAssignment Per review feedback from @beckerhe: move the negative offset/size validation from a precondition check in BufferAssignment::FromProto into BufferAllocation::AddAssignment itself, converting the existing CHECK_LE statements to TF_RET_CHECK. This protects every caller of AddAssignment (there are ~9 call sites across buffer_assignment.cc), not just the FromProto deserialization path, and returns an absl::Status error instead of crashing the process when the precondition is violated. Also adds explicit offset >= 0 / size >= 0 checks: converting CHECK_LE(offset, size_) to TF_RET_CHECK alone would not catch a negative offset, since offset <= size_ is still true for any negative offset and non-negative allocation size. All 9 existing call sites already propagate the returned Status via RETURN_IF_ERROR, so no call-site changes were needed beyond removing the now-redundant check in FromProto. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 5fdce18c4bdf2e1e53731611bc08938cf3669a05 PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
that referenced
this pull request
Jul 28, 2026
…:AddAssignment Imported from GitHub PR #46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ec by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. -- 5fdce18 by prasanna8585 <[email protected]>: Address review: move fix from FromProto to BufferAllocation::AddAssignment Per review feedback from @beckerhe: move the negative offset/size validation from a precondition check in BufferAssignment::FromProto into BufferAllocation::AddAssignment itself, converting the existing CHECK_LE statements to TF_RET_CHECK. This protects every caller of AddAssignment (there are ~9 call sites across buffer_assignment.cc), not just the FromProto deserialization path, and returns an absl::Status error instead of crashing the process when the precondition is violated. Also adds explicit offset >= 0 / size >= 0 checks: converting CHECK_LE(offset, size_) to TF_RET_CHECK alone would not catch a negative offset, since offset <= size_ is still true for any negative offset and non-negative allocation size. All 9 existing call sites already propagate the returned Status via RETURN_IF_ERROR, so no call-site changes were needed beyond removing the now-redundant check in FromProto. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 5fdce18 PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Jul 28, 2026
…:AddAssignment Imported from GitHub PR openxla/xla#46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ecd47988e30cfe195becfaecdb641479e8c by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. -- 5fdce18c4bdf2e1e53731611bc08938cf3669a05 by prasanna8585 <[email protected]>: Address review: move fix from FromProto to BufferAllocation::AddAssignment Per review feedback from @beckerhe: move the negative offset/size validation from a precondition check in BufferAssignment::FromProto into BufferAllocation::AddAssignment itself, converting the existing CHECK_LE statements to TF_RET_CHECK. This protects every caller of AddAssignment (there are ~9 call sites across buffer_assignment.cc), not just the FromProto deserialization path, and returns an absl::Status error instead of crashing the process when the precondition is violated. Also adds explicit offset >= 0 / size >= 0 checks: converting CHECK_LE(offset, size_) to TF_RET_CHECK alone would not catch a negative offset, since offset <= size_ is still true for any negative offset and non-negative allocation size. All 9 existing call sites already propagate the returned Status via RETURN_IF_ERROR, so no call-site changes were needed beyond removing the now-redundant check in FromProto. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 5fdce18c4bdf2e1e53731611bc08938cf3669a05 PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
that referenced
this pull request
Jul 28, 2026
…:AddAssignment Imported from GitHub PR #46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ec by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. -- 5fdce18 by prasanna8585 <[email protected]>: Address review: move fix from FromProto to BufferAllocation::AddAssignment Per review feedback from @beckerhe: move the negative offset/size validation from a precondition check in BufferAssignment::FromProto into BufferAllocation::AddAssignment itself, converting the existing CHECK_LE statements to TF_RET_CHECK. This protects every caller of AddAssignment (there are ~9 call sites across buffer_assignment.cc), not just the FromProto deserialization path, and returns an absl::Status error instead of crashing the process when the precondition is violated. Also adds explicit offset >= 0 / size >= 0 checks: converting CHECK_LE(offset, size_) to TF_RET_CHECK alone would not catch a negative offset, since offset <= size_ is still true for any negative offset and non-negative allocation size. All 9 existing call sites already propagate the returned Status via RETURN_IF_ERROR, so no call-site changes were needed beyond removing the now-redundant check in FromProto. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 5fdce18 PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Jul 28, 2026
…:AddAssignment Imported from GitHub PR openxla/xla#46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ecd47988e30cfe195becfaecdb641479e8c by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. -- 5fdce18c4bdf2e1e53731611bc08938cf3669a05 by prasanna8585 <[email protected]>: Address review: move fix from FromProto to BufferAllocation::AddAssignment Per review feedback from @beckerhe: move the negative offset/size validation from a precondition check in BufferAssignment::FromProto into BufferAllocation::AddAssignment itself, converting the existing CHECK_LE statements to TF_RET_CHECK. This protects every caller of AddAssignment (there are ~9 call sites across buffer_assignment.cc), not just the FromProto deserialization path, and returns an absl::Status error instead of crashing the process when the precondition is violated. Also adds explicit offset >= 0 / size >= 0 checks: converting CHECK_LE(offset, size_) to TF_RET_CHECK alone would not catch a negative offset, since offset <= size_ is still true for any negative offset and non-negative allocation size. All 9 existing call sites already propagate the returned Status via RETURN_IF_ERROR, so no call-site changes were needed beyond removing the now-redundant check in FromProto. Merging this change closes #46105 FUTURE_COPYBARA_INTEGRATE_REVIEW=openxla/xla#46105 from prasanna8585:fix/buffer-assignment-from-proto-negative-offset 5fdce18c4bdf2e1e53731611bc08938cf3669a05 PiperOrigin-RevId: 952638612
copybara-service Bot
pushed a commit
to tensorflow/tensorflow
that referenced
this pull request
Jul 28, 2026
…:AddAssignment Imported from GitHub PR openxla/xla#46105 ## Problem `BufferAssignment::FromProto` passes `assignee.offset()` and `assignee.size()` from a deserialized `BufferAllocationProto.Assigned` proto directly to `BufferAllocation::AddAssignment` without checking for negative values. The guards inside `AddAssignment` are: ```cpp CHECK_LE(offset, size_) // checks offset <= allocation.size() CHECK_LE(offset + size, size_) // checks offset + size <= allocation.size() ``` These `CHECK_LE` guards are **bypassed by negative int64 values**: `-1 <= 1024` is true for any real allocation, so a negative offset passes both checks and gets stored in `assigned_buffers_`. The stored negative offset is later used in LLVM IR codegen (`ir_emitter.cc`): ```cpp tempbuf_address_base + b()->getInt64(slice.offset()) ``` With `offset = -1`, this produces a pointer one byte before the allocation — an out-of-bounds address. ## Attack Surface Reachable from `CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)` where `serialized_aot_result` is a user-supplied serialized AOT executable. The proto fields `offset` and `size` are both `int64` in `hlo.proto`, so negative values are fully valid at the wire level. ## Relationship to PR #44653 PR #44653 (`2840cd3`) fixed `Slice::FromProto` with: ```cpp if (proto.offset() < 0 || proto.size() < 0) { return absl::OutOfRangeError(...); } ``` This PR applies the **same fix** to `BufferAssignment::FromProto` → `AddAssignment` path, which handles `Assigned` buffers in `BufferAllocationProto` — the sibling that the previous fix missed. ## Fix Added a pre-validation check in `BufferAssignment::FromProto` before calling `AddAssignment`: ```cpp if (assignee.offset() < 0 || assignee.size() < 0) { return absl::OutOfRangeError(absl::StrCat( "BufferAssignment proto contains an assigned buffer with negative ", "offset or size: logical_buffer_id=", logical_buffer_id, " offset=", assignee.offset(), " size=", assignee.size())); } ``` ## Testing Two new test cases in `buffer_assignment_test.cc`: - `BufferAssignmentFromProtoTest.RejectsNegativeOffset`: confirms `offset=-1` bypasses `CHECK_LE` guards (proving the vulnerability) and documents that the fix must reject it before reaching `AddAssignment` - `BufferAssignmentFromProtoTest.RejectsNegativeSize`: same for `size=-1` --- *Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.* Copybara import of the project: -- 6fab2ecd47988e30cfe195becfaecdb641479e8c by prasanna8585 <[email protected]>: [XLA] Validate assigned buffer offset/size in BufferAssignment::FromProto BufferAssignment::FromProto passes assignee.offset() and assignee.size() from a deserialized BufferAllocationProto.Assigned proto directly to BufferAllocation::AddAssignment without checking for negative values. The CHECK_LE guards in AddAssignment are bypassed by negative int64 values: CHECK_LE(-1, 1024) = true (for any real allocation) CHECK_LE(-1 + 4, 1024) = true So a negative offset is stored in assigned_buffers_ and later used in LLVM IR codegen (ir_emitter.cc) as: tempbuf_address_base + b()->getInt64(slice.offset()) With offset=-1 this is an out-of-bounds address. This mirrors the fix applied to Slice::FromProto in PR #44653 (2840cd3), which fixed the thunk deserialization path but missed this sibling path in BufferAssignment::FromProto. Fix: add an explicit negative check before calling AddAssignment, returning absl::OutOfRangeError for negative offset or size values. Add two regression tests confirming the bypass behavior and documenting the required fix behavior. -- 5fdce18c4bdf2e1e53731611bc08938cf3669a05 by prasanna8585 <[email protected]>: Address review: move fix from FromProto to BufferAllocation::AddAssignment Per review feedback from @beckerhe: move the negative offset/size validation from a precondition check in BufferAssignment::FromProto into BufferAllocation::AddAssignment itself, converting the existing CHECK_LE statements to TF_RET_CHECK. This protects every caller of AddAssignment (there are ~9 call sites across buffer_assignment.cc), not just the FromProto deserialization path, and returns an absl::Status error instead of crashing the process when the precondition is violated. Also adds explicit offset >= 0 / size >= 0 checks: converting CHECK_LE(offset, size_) to TF_RET_CHECK alone would not catch a negative offset, since offset <= size_ is still true for any negative offset and non-negative allocation size. All 9 existing call sites already propagate the returned Status via RETURN_IF_ERROR, so no call-site changes were needed beyond removing the now-redundant check in FromProto. Merging this change closes #46105 PiperOrigin-RevId: 955212759
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
BufferAssignment::FromProtopassesassignee.offset()andassignee.size()from a deserializedBufferAllocationProto.Assignedproto directly toBufferAllocation::AddAssignmentwithout checking for negative values.The guards inside
AddAssignmentare:These
CHECK_LEguards are bypassed by negative int64 values:-1 <= 1024is true for any real allocation, so a negative offset passes both checks and gets stored inassigned_buffers_.The stored negative offset is later used in LLVM IR codegen (
ir_emitter.cc):tempbuf_address_base + b()->getInt64(slice.offset())With
offset = -1, this produces a pointer one byte before the allocation — an out-of-bounds address.Attack Surface
Reachable from
CpuAotLoader::LoadAotCompilationResult(serialized_aot_result)whereserialized_aot_resultis a user-supplied serialized AOT executable. The proto fieldsoffsetandsizeare bothint64inhlo.proto, so negative values are fully valid at the wire level.Relationship to PR #44653
PR #44653 (
2840cd3) fixedSlice::FromProtowith:This PR applies the same fix to
BufferAssignment::FromProto→AddAssignmentpath, which handlesAssignedbuffers inBufferAllocationProto— the sibling that the previous fix missed.Fix
Added a pre-validation check in
BufferAssignment::FromProtobefore callingAddAssignment:Testing
Two new test cases in
buffer_assignment_test.cc:BufferAssignmentFromProtoTest.RejectsNegativeOffset: confirmsoffset=-1bypassesCHECK_LEguards (proving the vulnerability) and documents that the fix must reject it before reachingAddAssignmentBufferAssignmentFromProtoTest.RejectsNegativeSize: same forsize=-1Disclosure: this contribution was authored with the aid of an AI coding assistant and reviewed before submission.