Skip to content

Commit

Permalink
Prevent large step-size parameters (#1583)
Browse files Browse the repository at this point in the history
## Issue Addressed

Malicious users could request very large block ranges, more than we expect. Although technically legal, we are now quadraticaly weighting large step sizes in the filter. Therefore users may request large skips, but not a large number of blocks, to prevent requests forcing us to do long chain lookups. 

## Proposed Changes

Weight the step parameter in the RPC filter and prevent any overflows that effect us in the step parameter.

## Additional Info
  • Loading branch information
AgeManning committed Sep 11, 2020
1 parent 7f1b936 commit c6abc56
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 15 deletions.
25 changes: 24 additions & 1 deletion beacon_node/eth2_libp2p/src/rpc/rate_limiter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,30 @@ impl RPCRateLimiter {
request: &RPCRequest<T>,
) -> Result<(), RateLimitedErr> {
let time_since_start = self.init_time.elapsed();
let tokens = request.expected_responses().max(1);
let mut tokens = request.expected_responses().max(1);

// Increase the rate limit for blocks by range requests with large step counts.
// We count to tokens as a quadratic increase with step size.
// Using (step_size/5)^2 + 1 as penalty factor allows step sizes of 1-4 to have no penalty
// but step sizes higher than this add a quadratic penalty.
// Penalty's go:
// Step size | Penalty Factor
// 1 | 1
// 2 | 1
// 3 | 1
// 4 | 1
// 5 | 2
// 6 | 2
// 7 | 2
// 8 | 3
// 9 | 4
// 10 | 5

if let RPCRequest::BlocksByRange(bbr_req) = request {
let penalty_factor = (bbr_req.step as f64 / 5.0).powi(2) as u64 + 1;
tokens *= penalty_factor;
}

let check =
|limiter: &mut Limiter<PeerId>| limiter.allows(time_since_start, peer_id, tokens);
let limiter = match request.protocol() {
Expand Down
35 changes: 21 additions & 14 deletions beacon_node/network/src/router/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,22 +374,29 @@ impl<T: BeaconChainTypes> Processor<T> {
}
};

// pick out the required blocks, ignoring skip-slots and stepping by the step parameter;
// Pick out the required blocks, ignoring skip-slots and stepping by the step parameter.
//
// NOTE: We don't mind if req.count * req.step overflows as it just ends the iterator early and
// the peer will get less blocks.
// The step parameter is quadratically weighted in the filter, so large values should be
// prevented before reaching this point.
let mut last_block_root = None;
let maybe_block_roots = process_results(forwards_block_root_iter, |iter| {
iter.take_while(|(_, slot)| slot.as_u64() < req.start_slot + req.count * req.step)
// map skip slots to None
.map(|(root, _)| {
let result = if Some(root) == last_block_root {
None
} else {
Some(root)
};
last_block_root = Some(root);
result
})
.step_by(req.step as usize)
.collect::<Vec<Option<Hash256>>>()
iter.take_while(|(_, slot)| {
slot.as_u64() < req.start_slot.saturating_add(req.count * req.step)
})
// map skip slots to None
.map(|(root, _)| {
let result = if Some(root) == last_block_root {
None
} else {
Some(root)
};
last_block_root = Some(root);
result
})
.step_by(req.step as usize)
.collect::<Vec<Option<Hash256>>>()
});

let block_roots = match maybe_block_roots {
Expand Down

0 comments on commit c6abc56

Please sign in to comment.