Repository navigation
Conversation
The faucet method accepted user_id as an explicit argument, letting any caller pass an arbitrary user_id. This had two consequences: 1. Griefing: an attacker could call faucet(victim_id, max_amount) to exhaust the victim's rate-limit window without the victim's consent. The victim cannot claim from the faucet again until the 120-block window expires. 2. Front-running: a caller could manipulate another user's window_start_block by calling faucet(victim_id, 1) at any block, resetting the window to an arbitrary point. Fix: replace the user_id parameter with get_user_id(), which reads the authenticated caller ID from the execution context. The invoke_deferred call is unchanged since user_id now always refers to the caller.
Author
|
Note: the fix was verified by code review against the existing usage of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
faucet(user_id, amount)acceptsuser_idas an explicit argument, so any caller can pass an arbitrary user ID. This creates two issues:Griefing: an attacker calls
faucet(victim_id, max_amount)to exhaust the victim's entire rate-limit window. The victim receives the tokens but cannot claim from the faucet again for 120 blocks, without having requested anything.Window manipulation: an attacker calls
faucet(victim_id, 1)at an arbitrary block to reset the victim'swindow_start_block, shifting the 120-block window to a time the victim did not choose.In both cases the attack costs only a transaction fee.
Fix
Replace the
user_idparameter withget_user_id(), which reads the authenticated caller ID from the execution context. Theinvoke_deferredcall is unchanged —user_idnow always refers to the actual caller.Testing
Existing behavior is preserved for the normal case (caller claims for themselves). The griefing path is no longer reachable because
user_idis fixed to the transaction signer.