Skip to content

Commit 5d47d9d

Browse files
authored
feat(gossipsub): port 55e4a64 onto master
Pull-Request: #6359.
1 parent b6b79b2 commit 5d47d9d

5 files changed

Lines changed: 64 additions & 25 deletions

File tree

protocols/gossipsub/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@
5757
- Avoid direct casting from u128 to u64.
5858
See [PR 6211](https://github.com/libp2p/rust-libp2p/pull/6211).
5959

60+
## 0.49.4
61+
- Harden time arithmetic and bound remote PRUNE backoff.
62+
See [CVE](https://github.com/libp2p/rust-libp2p/security/advisories/GHSA-xqmp-fxgv-xvq5)
63+
6064
## 0.49.3
6165
- Ignore invalid backoff values on peer prune.
6266
See [CVE GHSA-gc42-3jg7-rxr2](https://github.com/libp2p/rust-libp2p/security/advisories/GHSA-gc42-3jg7-rxr2)

protocols/gossipsub/src/backoff.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,10 @@ impl BackoffStorage {
158158
let now = Instant::now();
159159
s.retain(|(topic, peer)| {
160160
let keep = match Self::get_backoff_time_from_backoffs(backoffs, topic, peer) {
161-
Some(backoff_time) => backoff_time + slack > now,
161+
Some(backoff_time) => backoff_time
162+
.checked_add(slack)
163+
.map(|backoff| backoff > now)
164+
.unwrap_or(false),
162165
None => false,
163166
};
164167
if !keep {

protocols/gossipsub/src/behaviour.rs

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,9 @@ const IDONTWANT_CAP: usize = 10_000;
9292
/// IDONTWANT timeout before removal.
9393
const IDONTWANT_TIMEOUT: Duration = Duration::new(3, 0);
9494

95+
/// Max allowed PRUNE backoff, 1 hour.
96+
const MAX_REMOTE_PRUNE_BACKOFF_SECONDS: u64 = 3600;
97+
9598
/// Determines if published messages should be signed or not.
9699
///
97100
/// Without signing, a number of privacy preserving modes can be selected.
@@ -1414,11 +1417,14 @@ where
14141417
iwant_ids_vec.truncate(iask);
14151418
*iasked += iask;
14161419

1417-
self.gossip_promises.add_promise(
1418-
*peer_id,
1419-
&iwant_ids_vec,
1420-
Instant::now() + self.config.iwant_followup_time(),
1421-
);
1420+
let followup_time = Instant::now()
1421+
.checked_add(self.config.iwant_followup_time())
1422+
.unwrap_or_else(|| {
1423+
tracing::error!("Invalid iwant_followup_time using Instant::now()");
1424+
Instant::now()
1425+
});
1426+
self.gossip_promises
1427+
.add_promise(*peer_id, &iwant_ids_vec, followup_time);
14221428
tracing::trace!(
14231429
peer=%peer_id,
14241430
"IHAVE: Asking for the following messages from peer: {:?}",
@@ -1553,11 +1559,26 @@ where
15531559
}
15541560
peer_score.add_penalty(peer_id, 1);
15551561

1556-
// check the flood cutoff
1557-
let flood_cutoff = (backoff_time + self.config.graft_flood_threshold())
1558-
- self.config.prune_backoff();
1559-
if flood_cutoff > now {
1560-
// extra penalty
1562+
// Apply an extra graft-backoff penalty only when the peer is still
1563+
// far enough from backoff expiry.
1564+
// This compares durations only,
1565+
// avoiding Instant arithmetic and handling config edge cases
1566+
// safely: any active backoff
1567+
// qualifies for the extra penalty.
1568+
let apply_extra_penalty = match self
1569+
.config
1570+
.prune_backoff()
1571+
.checked_sub(self.config.graft_flood_threshold())
1572+
{
1573+
Some(required_remaining) => {
1574+
let remaining_backoff =
1575+
backoff_time.saturating_duration_since(now);
1576+
remaining_backoff > required_remaining
1577+
}
1578+
// graft_flood_threshold >= prune_backoff
1579+
None => true,
1580+
};
1581+
if apply_extra_penalty {
15611582
peer_score.add_penalty(peer_id, 1);
15621583
}
15631584
}
@@ -1708,10 +1729,11 @@ where
17081729
}
17091730
}
17101731
if always_update_backoff || peer_removed {
1711-
let time = if let Some(backoff) = backoff {
1712-
Duration::from_secs(backoff)
1713-
} else {
1714-
self.config.prune_backoff()
1732+
let time = match backoff {
1733+
Some(backoff) => {
1734+
Duration::from_secs(std::cmp::min(backoff, MAX_REMOTE_PRUNE_BACKOFF_SECONDS))
1735+
}
1736+
None => self.config.prune_backoff(),
17151737
};
17161738
// is there a backoff specified by the peer? if so obey it.
17171739
self.backoffs.update_backoff(topic_hash, peer_id, time);
@@ -2589,7 +2611,7 @@ where
25892611
let fanout = &mut self.fanout; // help the borrow checker
25902612
let fanout_ttl = self.config.fanout_ttl();
25912613
self.fanout_last_pub.retain(|topic_hash, last_pub_time| {
2592-
if *last_pub_time + fanout_ttl < Instant::now() {
2614+
if fanout_ttl < Instant::now().saturating_duration_since(*last_pub_time) {
25932615
tracing::debug!(
25942616
topic=%topic_hash,
25952617
"HEARTBEAT: Fanout topic removed due to timeout"
@@ -2704,7 +2726,7 @@ where
27042726
// Flush stale IDONTWANTs.
27052727
for peer in self.connected_peers.values_mut() {
27062728
while let Some((_front, instant)) = peer.dont_send.front() {
2707-
if (*instant + IDONTWANT_TIMEOUT) >= Instant::now() {
2729+
if IDONTWANT_TIMEOUT >= Instant::now().saturating_duration_since(*instant) {
27082730
break;
27092731
} else {
27102732
peer.dont_send.pop_front();

protocols/gossipsub/src/peer_score.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -606,9 +606,13 @@ impl PeerScore {
606606
topic_stats.mesh_message_deliveries_active = false;
607607
}
608608

609-
peer_stats.status = ConnectionStatus::Disconnected {
610-
expire: Instant::now() + self.params.retain_score,
611-
};
609+
let expire = Instant::now()
610+
.checked_add(self.params.retain_score)
611+
.unwrap_or_else(|| {
612+
tracing::error!("invalid retain_score value, using Instant::now()");
613+
Instant::now()
614+
});
615+
peer_stats.status = ConnectionStatus::Disconnected { expire };
612616
}
613617
}
614618

protocols/gossipsub/src/time_cache.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,17 @@ where
140140
self.remove_expired_keys(now);
141141
match self.map.entry(key) {
142142
Occupied(entry) => Entry::Occupied(OccupiedEntry { entry }),
143-
Vacant(entry) => Entry::Vacant(VacantEntry {
144-
expiration: now + self.ttl,
145-
entry,
146-
list: &mut self.list,
147-
}),
143+
Vacant(entry) => {
144+
let expiration = now.checked_add(self.ttl).unwrap_or_else(|| {
145+
tracing::error!("invalid time cache ttl");
146+
now
147+
});
148+
Entry::Vacant(VacantEntry {
149+
expiration,
150+
entry,
151+
list: &mut self.list,
152+
})
153+
}
148154
}
149155
}
150156

0 commit comments

Comments
 (0)