Skip to content

Commit 077d435

Browse files
improve error handling in webRTC-related noise function (paritytech#377)
Closes paritytech#43 This PR: - [x] eliminates the unsafe assumption and panics in the WebRTC litep2p config when fetching `Multiaddr`. Ensures litep2p behaves predictably even with malformed or malicious peers during the handshake phase by introducing `InvalidMultihash` in the `AddressError`. - [x] removes assumption about the `reply` buffer size, by checking the length before splitting in `get_remote_public_key` function. I am also looking at other functions to see if there should be any improvement, i will make this a draft PR untill i finish other improvements or if i find no other improvements needed. --------- Co-authored-by: Dmitry Markin <[email protected]>
1 parent a950f7a commit 077d435

File tree

3 files changed

+13
-15
lines changed

3 files changed

+13
-15
lines changed

src/crypto/noise/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,11 @@ impl NoiseContext {
164164
/// Get remote public key from the received Noise payload.
165165
#[cfg(feature = "webrtc")]
166166
pub fn get_remote_public_key(&mut self, reply: &[u8]) -> Result<PublicKey, NegotiationError> {
167+
if reply.len() < 2 {
168+
tracing::error!(target: LOG_TARGET, "reply too short to contain length prefix");
169+
return Err(NegotiationError::ParseError(ParseError::InvalidReplyLength));
170+
}
171+
167172
let (len_slice, reply) = reply.split_at(2);
168173
let len = u16::from_be_bytes(
169174
len_slice

src/error.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ pub enum ParseError {
181181
/// This error is protocol specific.
182182
#[error("Invalid data")]
183183
InvalidData,
184+
/// The provided reply length is not valid
185+
#[error("Invalid reply length")]
186+
InvalidReplyLength,
184187
}
185188

186189
#[derive(Debug, thiserror::Error)]

src/lib.rs

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,13 @@ use crate::transport::websocket::WebSocketTransport;
5555

5656
use hickory_resolver::{name_server::TokioConnectionProvider, TokioResolver};
5757
use multiaddr::{Multiaddr, Protocol};
58-
use multihash::Multihash;
5958
use transport::Endpoint;
6059
use types::ConnectionId;
6160

62-
use std::{collections::HashSet, sync::Arc};
63-
6461
pub use bandwidth::BandwidthSink;
6562
pub use error::Error;
6663
pub use peer_id::PeerId;
64+
use std::{collections::HashSet, sync::Arc};
6765
pub use types::protocol::ProtocolName;
6866

6967
pub(crate) mod peer_id;
@@ -332,9 +330,7 @@ impl Litep2p {
332330

333331
for address in transport_listen_addresses {
334332
transport_manager.register_listen_address(address.clone());
335-
listen_addresses.push(address.with(Protocol::P2p(
336-
Multihash::from_bytes(&local_peer_id.to_bytes()).unwrap(),
337-
)));
333+
listen_addresses.push(address.with(Protocol::P2p(*local_peer_id.as_ref())));
338334
}
339335

340336
transport_manager.register_transport(SupportedTransport::Tcp, Box::new(transport));
@@ -349,9 +345,7 @@ impl Litep2p {
349345

350346
for address in transport_listen_addresses {
351347
transport_manager.register_listen_address(address.clone());
352-
listen_addresses.push(address.with(Protocol::P2p(
353-
Multihash::from_bytes(&local_peer_id.to_bytes()).unwrap(),
354-
)));
348+
listen_addresses.push(address.with(Protocol::P2p(*local_peer_id.as_ref())));
355349
}
356350

357351
transport_manager.register_transport(SupportedTransport::Quic, Box::new(transport));
@@ -366,9 +360,7 @@ impl Litep2p {
366360

367361
for address in transport_listen_addresses {
368362
transport_manager.register_listen_address(address.clone());
369-
listen_addresses.push(address.with(Protocol::P2p(
370-
Multihash::from_bytes(&local_peer_id.to_bytes()).unwrap(),
371-
)));
363+
listen_addresses.push(address.with(Protocol::P2p(*local_peer_id.as_ref())));
372364
}
373365

374366
transport_manager.register_transport(SupportedTransport::WebRtc, Box::new(transport));
@@ -383,9 +375,7 @@ impl Litep2p {
383375

384376
for address in transport_listen_addresses {
385377
transport_manager.register_listen_address(address.clone());
386-
listen_addresses.push(address.with(Protocol::P2p(
387-
Multihash::from_bytes(&local_peer_id.to_bytes()).unwrap(),
388-
)));
378+
listen_addresses.push(address.with(Protocol::P2p(*local_peer_id.as_ref())));
389379
}
390380

391381
transport_manager

0 commit comments

Comments
 (0)