Skip to content

Commit cf4682f

Browse files
committed
Simplify ordering on unordered interleaved streams
This fixes a vulnerability where an attacker could cause a panic by sending unordered interleaved stream fragments with Message Identifiers (MIDs) chosen exactly 2^31 apart. The unordered stream reassembler uses IntervalList, which relies on slice::partition_point. This standard library method requires a strict linear total order to perform binary search. RFC1982 sequence spaces are circular, meaning they violate total ordering at their maximum distance (e.g. A < B < C < A). When chunks arrive with MIDs exactly at this maximum boundary, partition_point fails its invariants and panics. Ordered streams are immune to this panic because they drop chunks that fall outside a strictly validated forward half-space window (via next_mid and is_valid_successor). This constrains their queued chunks to a narrow slice of the circle where the math never wraps around, allowing partition_point to sort them safely. Unordered streams, however, have no such delivery window and accept chunks from anywhere on the sequence circle. But for unordered chunks, the MIDs have no real ordering; The MID is used exclusively as a correlation ID to group fragments of the same message together. The relative order of different unordered MIDs is completely irrelevant. This commit introduces a dedicated UnorderedInterleavedKey that bypasses RFC1982 ordering and instead derives standard linear u32 ordering. This satisfies partition_point's strict total ordering requirement and prevents the vulnerability.
1 parent f648f61 commit cf4682f

2 files changed

Lines changed: 50 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ and this project adheres to
2323
- Discard FORWARD-TSN with invalid TSN.
2424
- Stop heartbeat timeout only after validating nonce in HEARTBEAT-ACK.
2525
- Dropping/truncating oversized Unrecognized or HEARTBEAT chunks.
26+
- Avoid panic on unordered interleaved streams when receiving crafted MIDs.
2627

2728
### Changed
2829

src/rx/interleaved_reassembly_streams.rs

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,35 @@ impl ReassemblyKey for InterleavedKey {
7474
}
7575
}
7676

77+
/// A reassembly key for unordered interleaved streams that derives standard linear ordering.
78+
///
79+
/// Unordered streams accept chunks with arbitrary MIDs without discarding them based on
80+
/// sequence bounds. To safely store these in an `IntervalList` (which requires a strict
81+
/// total order for binary search), bypass RFC1982 ordering and use standard sorting instead.
82+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
83+
pub struct UnorderedInterleavedKey {
84+
pub mid: Mid,
85+
pub fsn: Fsn,
86+
}
87+
88+
impl PartialOrd for UnorderedInterleavedKey {
89+
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
90+
Some(self.cmp(other))
91+
}
92+
}
93+
94+
impl Ord for UnorderedInterleavedKey {
95+
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
96+
self.mid.0.cmp(&other.mid.0).then_with(|| self.fsn.0.cmp(&other.fsn.0))
97+
}
98+
}
99+
100+
impl ReassemblyKey for UnorderedInterleavedKey {
101+
fn next(&self) -> Self {
102+
UnorderedInterleavedKey { mid: self.mid, fsn: self.fsn + 1 }
103+
}
104+
}
105+
77106
pub struct OrderedStream {
78107
stream_id: StreamId,
79108
intervals: IntervalList<InterleavedKey>,
@@ -127,7 +156,7 @@ impl OrderedStream {
127156

128157
pub struct UnorderedStream {
129158
stream_id: StreamId,
130-
intervals: IntervalList<InterleavedKey>,
159+
intervals: IntervalList<UnorderedInterleavedKey>,
131160
}
132161

133162
impl UnorderedStream {
@@ -142,7 +171,7 @@ impl UnorderedStream {
142171
return 0;
143172
}
144173

145-
let key = InterleavedKey { mid: data.mid, fsn: data.fsn };
174+
let key = UnorderedInterleavedKey { mid: data.mid, fsn: data.fsn };
146175
let queued_bytes = data.payload.len() as isize;
147176
let idx = self.intervals.add(key, data);
148177

@@ -653,4 +682,22 @@ mod tests {
653682
assert_eq!(messages.len(), 1);
654683
assert_eq!(messages[0].payload, b"efgh");
655684
}
685+
686+
#[test]
687+
fn unordered_interleaved_streams_support_extreme_mid_distances() {
688+
let mut streams = InterleavedReassemblyStreams::new();
689+
let mut seq = DataSequencer::new(StreamId(1));
690+
691+
let mut data1 = seq.unordered("a", "B");
692+
data1.mid = Mid(0);
693+
streams.add(Tsn(1), data1, &mut |_| {});
694+
695+
let mut data2 = seq.unordered("b", "B");
696+
data2.mid = Mid(1 << 31);
697+
streams.add(Tsn(2), data2, &mut |_| {});
698+
699+
// Ensure that binary search invariants in IntervalList are maintained
700+
// even when receiving MIDs exactly at the maximum sequence distance.
701+
assert_eq!(streams.queued_bytes(), 2);
702+
}
656703
}

0 commit comments

Comments
 (0)