sherlock-audit / 2023-10-mzero-judging

3 stars 2 forks source link

levi_104 - Using ordered nonces may lead to short-term DoS. #16

Closed sherlock-admin4 closed 7 months ago

sherlock-admin4 commented 7 months ago

levi_104

high

Using ordered nonces may lead to short-term DoS.

Summary

Using ordered nonces may lead to short-term DoS.

Vulnerability Detail

When checking whether the nonce of the signature is valid (to prevent signature replay), a sequential approach was used.

    function _checkAndIncrementNonce(address account_, uint256 nonce_) internal {
        unchecked {
            uint256 currentNonce_ = nonces[account_]++; // Nonce realistically cannot overflow.

            if (nonce_ != currentNonce_) revert InvalidAccountNonce(nonce_, currentNonce_);
        }
    }

Using ordered nonces may lead to short-term DoS. There is an easy example: user1 gets the signature with nonce 1, but he doesn't call the delegateBySig(). In this time, user2 gets the signature with nonce 2. User2 call delegateBySig() but tx revert because his nonce is not the currentNonce_. User2 can only call delegateBySig() after user1.

Impact

Users can only call delegateBySig() in order, and once a user delays the call, subsequent users are also affected.

Code Snippet

Manual Review

Recommendation

Using unordered nonce.

Duplicate of #12

PierrickGT commented 7 months ago

Invalid issue.

Nonces are per user and not global.

sherlock-admin4 commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

this seem valid; even if user refuses to call it at all, others will also be affected; medium(6)

nevillehuang commented 7 months ago

Agree with sponsors comment here