The rate-limiter is called prior to application-level checks on request validity, so we should be careful to avoid integer overflow in its code, as it is operating on untrusted data.

There is integer overflow here:

The tokens value is derived from untrusted user input:

The count from the request is untrusted and can be set arbitrarily:

This was briefly fixed for BlobsByRange requests on unstable when we implemented a size check during decoding, but will regress again when this PR is merged:

• Fork aware max values in rpc #6847

The fix that existed in unstable was coincidental, so I think it is better if we address the root cause and avoid overflowing integer arithmetic in the rate limiter altogether. We can probably replace most uses of +, -, * by their saturating_ equivalents to get the right semantics.

We can also enable the arithmetic_side_effects lint, which we use in consensus crates. See:

• Expand checked arithmetic to more crates #1013

This issue is not deemed security-sensitive because other mechanisms exist to defend the node from peers that would bypass the rate limiter. Any request with an invalid count will be rejected by application-level checks and result in the peer being quickly banned.

Thanks to @gln for reporting this bug as part of the Immunefi Ethereum Attackathon.