sherlock-audit / 2024-05-pooltogether-judging

13 stars 8 forks source link

0xRajkumar - When allowance is not equal to the assets in Prizevault, it causes a DOS. #135

Closed sherlock-admin3 closed 5 months ago

sherlock-admin3 commented 5 months ago

0xRajkumar

medium

When allowance is not equal to the assets in Prizevault, it causes a DOS.

Summary

In the depositwithpermit function, we mitigate the risk of DOS by checking if the allowance is equal to the assets or not. However, what if there was an allowance before using the permit? Let's consider a scenario where the user had a nonzero allowance, and then they used depositwithpermit by providing a signature. If an attacker had already used the permit before the user, the allowance would not be equal to the assets. Consequently, we would end up using the permit again with the same signature, leading to a DOS situation.

Vulnerability Detail

Scenario:

The user had an allowance equal to 1 wei before calling depositwithpermit. The user then calls depositwithpermit with a signature using the _assets amount. Before this transaction executes, the attacker extracts the signature and calls the permit beforehand. Now, in this case, our _assets will not be equal to the allowance because the allowance will be 1 wei more, leading us to execute the permit, which will cause a DOS.

Impact

It will clearly cause a DOS when the user had some allowance before calling the depositwithpermit function.

Code Snippet

        if (_asset.allowance(_owner, address(this)) != _assets) {
            IERC20Permit(address(_asset)).permit(
                _owner,
                address(this),
                _assets,
                _deadline,
                _v,
                _r,
                _s
            );
        }

References

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-vault/src/PrizeVault.sol#L530

Tool used

Manual Review

Recommendation

We should use < instead of != like this:

        if (_asset.allowance(_owner, address(this)) < _assets) {
            IERC20Permit(address(_asset)).permit(
                _owner,
                address(this),
                _assets,
                _deadline,
                _v,
                _r,
                _s
            );
        }

Duplicate of #89

sherlock-admin4 commented 5 months ago

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

infect3d commented:

calling permit will not add assets to the allowance but write over previous allowance