santiagomed opened a new pull request, #3335:
URL: https://github.com/apache/thrift/pull/3335
## Problem
`TCompactInputProtocol::read_field_begin` accumulates field ID deltas into
`last_read_field_id: i16` with an unchecked addition:
```rust
self.last_read_field_id += field_delta as i16;
```
Each delta is encoded in the high nibble of a field header byte (range
1–15). A crafted input can advance the accumulator past `i16::MAX` in as few as
two fields.
**With `-C overflow-checks`** (enabled by `cargo fuzz`): panics, aborting
the process.
**In release builds** (no overflow check): the accumulator silently wraps to
a large negative value. All subsequent fields in the struct fall into the
wildcard `_` arm and are silently discarded — no error is surfaced.
In structs carrying cryptographic material (e.g. key versions, participant
public keys) this means security-critical fields can be silently dropped
without any indication of tampering.
## Fix
Replace the unchecked `+=` with `i16::checked_add`, returning a
`ProtocolError` on overflow:
```rust
self.last_read_field_id = self
.last_read_field_id
.checked_add(field_delta as i16)
.ok_or_else(|| {
crate::Error::Protocol(ProtocolError::new(
ProtocolErrorKind::InvalidData,
"field id overflow",
))
})?;
```
## Testing
All existing Rust library tests pass. The fix was found and validated
through fuzzing infrastructure at xAI, where we use the Thrift Rust library in
production messaging systems.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
