Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Commit 9a0d747

Browse files
behzadnourimergify[bot]
authored andcommitted
removes raw indexing into packet data (#25554)
Packets are at the boundary of the system where, vast majority of the time, they are received from an untrusted source. Raw indexing into the data buffer can open attack vectors if the offsets are invalid. Validating offsets beforehand is verbose and error prone. The commit updates Packet::data() api to take a SliceIndex and always to return an Option. The call-sites are so forced to explicitly handle the case where the offsets are invalid. (cherry picked from commit 5dbf7d8) # Conflicts: # ledger/src/shred.rs # ledger/src/sigverify_shreds.rs # perf/src/sigverify.rs
1 parent 529b856 commit 9a0d747

File tree

15 files changed

+435
-107
lines changed

15 files changed

+435
-107
lines changed

bench-streamer/src/main.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ fn producer(addr: &SocketAddr, exit: Arc<AtomicBool>) -> JoinHandle<()> {
3737
for p in packet_batch.iter() {
3838
let a = p.meta.socket_addr();
3939
assert!(p.meta.size <= PACKET_DATA_SIZE);
40-
send.send_to(p.data(), &a).unwrap();
40+
let data = p.data(..).unwrap_or_default();
41+
send.send_to(data, &a).unwrap();
4142
num += 1;
4243
}
4344
assert_eq!(num, 10);

core/src/banking_stage.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ impl BankingStage {
583583
.iter()
584584
.filter_map(|p| {
585585
if !p.meta.forwarded() && data_budget.take(p.meta.size) {
586-
Some(p.data().to_vec())
586+
Some(p.data(..)?.to_vec())
587587
} else {
588588
None
589589
}

core/src/packet_hasher.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ impl Default for PacketHasher {
2626

2727
impl PacketHasher {
2828
pub(crate) fn hash_packet(&self, packet: &Packet) -> u64 {
29-
self.hash_data(packet.data())
29+
self.hash_data(packet.data(..).unwrap_or_default())
3030
}
3131

3232
pub(crate) fn hash_shred(&self, shred: &Shred) -> u64 {

core/src/serve_repair.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -812,7 +812,7 @@ mod tests {
812812
.into_iter()
813813
.filter_map(|p| {
814814
assert_eq!(repair_response::nonce(p).unwrap(), nonce);
815-
Shred::new_from_serialized_shred(p.data().to_vec()).ok()
815+
Shred::new_from_serialized_shred(p.data(..).unwrap().to_vec()).ok()
816816
})
817817
.collect();
818818
assert!(!rv.is_empty());
@@ -896,7 +896,7 @@ mod tests {
896896
.into_iter()
897897
.filter_map(|p| {
898898
assert_eq!(repair_response::nonce(p).unwrap(), nonce);
899-
Shred::new_from_serialized_shred(p.data().to_vec()).ok()
899+
Shred::new_from_serialized_shred(p.data(..).unwrap().to_vec()).ok()
900900
})
901901
.collect();
902902
assert_eq!(rv[0].index(), 1);
@@ -1348,7 +1348,7 @@ mod tests {
13481348

13491349
fn verify_responses<'a>(request: &ShredRepairType, packets: impl Iterator<Item = &'a Packet>) {
13501350
for packet in packets {
1351-
let shred_payload = packet.data().to_vec();
1351+
let shred_payload = packet.data(..).unwrap().to_vec();
13521352
let shred = Shred::new_from_serialized_shred(shred_payload).unwrap();
13531353
request.verify_response(&shred);
13541354
}

core/src/unprocessed_packet_batches.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -377,12 +377,14 @@ pub fn deserialize_packets<'a>(
377377

378378
/// Read the transaction message from packet data
379379
pub fn packet_message(packet: &Packet) -> Result<&[u8], DeserializedPacketError> {
380-
let (sig_len, sig_size) =
381-
decode_shortu16_len(packet.data()).map_err(DeserializedPacketError::ShortVecError)?;
380+
let (sig_len, sig_size) = packet
381+
.data(..)
382+
.and_then(|bytes| decode_shortu16_len(bytes).ok())
383+
.ok_or(DeserializedPacketError::ShortVecError(()))?;
382384
sig_len
383385
.checked_mul(size_of::<Signature>())
384386
.and_then(|v| v.checked_add(sig_size))
385-
.and_then(|msg_start| packet.data().get(msg_start..))
387+
.and_then(|msg_start| packet.data(msg_start..))
386388
.ok_or(DeserializedPacketError::SignatureOverflowed(sig_size))
387389
}
388390

core/src/window_service.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ where
363363
inc_new_counter_debug!("streamer-recv_window-invalid_or_unnecessary_packet", 1);
364364
return None;
365365
}
366-
let serialized_shred = packet.data().to_vec();
366+
let serialized_shred = packet.data(..)?.to_vec();
367367
let shred = Shred::new_from_serialized_shred(serialized_shred).ok()?;
368368
if !shred_filter(&shred, working_bank.clone(), last_root) {
369369
return None;

gossip/tests/gossip.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ pub fn cluster_info_retransmit() {
260260
let retransmit_peers: Vec<_> = peers.iter().collect();
261261
retransmit_to(
262262
&retransmit_peers,
263-
p.data(),
263+
p.data(..).unwrap(),
264264
&tn1,
265265
false,
266266
&SocketAddrSpace::Unspecified,

0 commit comments

Comments
 (0)