Prevent large step-size parameters (#1583)

## 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
This commit is contained in:
Age Manning 2020-09-11 02:33:36 +00:00
parent 7f1b936905
commit c6abc56113
2 changed files with 45 additions and 15 deletions

View File

@ -189,7 +189,30 @@ impl RPCRateLimiter {
request: &RPCRequest<T>, request: &RPCRequest<T>,
) -> Result<(), RateLimitedErr> { ) -> Result<(), RateLimitedErr> {
let time_since_start = self.init_time.elapsed(); 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 = let check =
|limiter: &mut Limiter<PeerId>| limiter.allows(time_since_start, peer_id, tokens); |limiter: &mut Limiter<PeerId>| limiter.allows(time_since_start, peer_id, tokens);
let limiter = match request.protocol() { let limiter = match request.protocol() {

View File

@ -374,10 +374,17 @@ 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 mut last_block_root = None;
let maybe_block_roots = process_results(forwards_block_root_iter, |iter| { 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) iter.take_while(|(_, slot)| {
slot.as_u64() < req.start_slot.saturating_add(req.count * req.step)
})
// map skip slots to None // map skip slots to None
.map(|(root, _)| { .map(|(root, _)| {
let result = if Some(root) == last_block_root { let result = if Some(root) == last_block_root {