Skip to content

Commit 77a9916

Browse files
committed
fix(yamux): protect from overflow vulnerability
1 parent 7a942be commit 77a9916

File tree

6 files changed

+40
-11
lines changed

6 files changed

+40
-11
lines changed

p2p/src/network/scheduler/p2p_network_scheduler_actions.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ pub enum P2pNetworkSchedulerAction {
7272
addr: ConnectionAddr,
7373
peer_id: PeerId,
7474
message_size_limit: Limit<usize>,
75+
pending_outgoing_limit: Limit<usize>,
7576
},
7677

7778
/// Action that initiate the specified peer disconnection.

p2p/src/network/scheduler/p2p_network_scheduler_reducer.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,9 @@ impl P2pNetworkSchedulerState {
249249
}
250250
P2pNetworkSchedulerAction::YamuxDidInit {
251251
addr,
252-
message_size_limit,
253252
peer_id,
253+
message_size_limit,
254+
pending_outgoing_limit,
254255
} => {
255256
let Some(cn) = scheduler_state.connections.get_mut(&addr) else {
256257
bug_condition!(
@@ -261,6 +262,7 @@ impl P2pNetworkSchedulerState {
261262
if let Some(P2pNetworkConnectionMuxState::Yamux(yamux)) = &mut cn.mux {
262263
yamux.init = true;
263264
yamux.message_size_limit = message_size_limit;
265+
yamux.pending_outgoing_limit = pending_outgoing_limit;
264266
}
265267

266268
let incoming = cn.incoming;
@@ -503,10 +505,13 @@ impl P2pNetworkSchedulerState {
503505
return;
504506
};
505507
let message_size_limit = p2p_state.config.limits.yamux_message_size();
508+
let pending_outgoing_limit =
509+
p2p_state.config.limits.yamux_pending_outgoing_per_peer();
506510
dispatcher.push(P2pNetworkSchedulerAction::YamuxDidInit {
507511
addr,
508512
peer_id,
509513
message_size_limit,
514+
pending_outgoing_limit,
510515
});
511516
}
512517
Some(Protocol::Stream(kind)) => {

p2p/src/network/scheduler/p2p_network_scheduler_state.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,8 @@ pub enum P2pNetworkConnectionError {
196196
StreamReset(StreamId),
197197
#[error("pubsub error: {0}")]
198198
PubSubError(String),
199+
#[error("peer make us keep too much data")]
200+
YamuxOverflow(StreamId),
199201
}
200202

201203
#[derive(Serialize, Deserialize, Debug, Clone)]

p2p/src/network/yamux/p2p_network_yamux_reducer.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -222,20 +222,15 @@ impl P2pNetworkYamuxState {
222222
if difference > 0 {
223223
// have some fresh space in the window
224224
// try send as many frames as can
225-
// last frame
226225
let mut window = stream.window_theirs;
227226
while let Some(mut frame) = stream.pending.pop_front() {
228-
let size = if let YamuxFrameInner::Data(data) = &frame.inner {
229-
data.len()
230-
} else {
231-
0
232-
} as u32;
233-
if let Some(new_window) = window.checked_sub(size) {
227+
let len = frame.len() as u32;
228+
if let Some(new_window) = window.checked_sub(len) {
234229
pending_outgoing.push_back(frame);
235230
window = new_window;
236231
} else {
237232
if let Some(remaining) =
238-
frame.split_at((size - window) as usize)
233+
frame.split_at((len - window) as usize)
239234
{
240235
stream.pending.push_front(remaining);
241236
}
@@ -360,7 +355,8 @@ impl P2pNetworkYamuxState {
360355
Ok(())
361356
}
362357
P2pNetworkYamuxAction::OutgoingFrame { mut frame, addr } => {
363-
let Some(stream) = yamux_state.streams.get_mut(&frame.stream_id) else {
358+
let stream_id = frame.stream_id;
359+
let Some(stream) = yamux_state.streams.get_mut(&stream_id) else {
364360
return Ok(());
365361
};
366362
match &mut frame.inner {
@@ -386,6 +382,14 @@ impl P2pNetworkYamuxState {
386382
// or the queue is already not empty
387383
// in both cases the whole frame goes in the queue and nothing to send
388384
stream.pending.push_back(frame);
385+
if stream.pending.iter().map(YamuxFrame::len).sum::<usize>()
386+
> yamux_state.pending_outgoing_limit
387+
{
388+
let dispatcher = state_context.into_dispatcher();
389+
let error = P2pNetworkConnectionError::YamuxOverflow(stream_id);
390+
dispatcher.push(P2pNetworkSchedulerAction::Error { addr, error });
391+
}
392+
389393
return Ok(());
390394
}
391395
}

p2p/src/network/yamux/p2p_network_yamux_state.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use super::super::*;
77
#[derive(Serialize, Deserialize, Debug, Clone, Default)]
88
pub struct P2pNetworkYamuxState {
99
pub message_size_limit: Limit<usize>,
10+
pub pending_outgoing_limit: Limit<usize>,
1011
pub buffer: Vec<u8>,
1112
pub incoming: VecDeque<YamuxFrame>,
1213
pub streams: BTreeMap<StreamId, YamuxStreamState>,
@@ -185,13 +186,21 @@ impl YamuxFrame {
185186
vec
186187
}
187188

189+
pub fn len(&self) -> usize {
190+
if let YamuxFrameInner::Data(data) = &self.inner {
191+
data.len()
192+
} else {
193+
0
194+
}
195+
}
196+
188197
/// If this data is bigger then `pos`, keep only first `pos` bytes and return some remaining
189198
/// otherwise return none
190199
pub fn split_at(&mut self, pos: usize) -> Option<Self> {
191200
use std::ops::Sub;
192201

193202
if let YamuxFrameInner::Data(data) = &mut self.inner {
194-
if data.len() == pos {
203+
if data.len() <= pos {
195204
return None;
196205
}
197206
let (keep, rest) = data.split_at(pos);

p2p/src/p2p_config.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ pub struct P2pLimits {
252252
max_peers_in_state: Limit<usize>,
253253
max_streams: Limit<usize>,
254254
yamux_message_size: Limit<usize>,
255+
yamux_pending_outgoing_per_peer: Limit<usize>,
255256

256257
identify_message: Limit<usize>,
257258
kademlia_request: Limit<usize>,
@@ -319,6 +320,12 @@ impl P2pLimits {
319320
/// Sets the maximum number of streams that a peer is allowed to open simultaneously.
320321
with_yamux_message_size
321322
);
323+
limit!(
324+
/// Maximum number of streams from a peer.
325+
yamux_pending_outgoing_per_peer,
326+
/// Sets the maximum number of streams that a peer is allowed to open simultaneously.
327+
with_yamux_pending_outgoing_per_peer
328+
);
322329

323330
limit!(
324331
/// Minimum number of peers.
@@ -400,6 +407,7 @@ impl Default for P2pLimits {
400407
max_peers_in_state,
401408
max_streams,
402409
yamux_message_size,
410+
yamux_pending_outgoing_per_peer: rpc_get_staged_ledger,
403411

404412
identify_message,
405413
kademlia_request,

0 commit comments

Comments
 (0)