Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

turvec - Attackers can cause DOS attacks for tokens with Cap #142

Closed
sherlock-admin3 opened this issue Aug 30, 2024 · 6 comments
Closed
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed

Comments

@sherlock-admin3
Copy link
Contributor

sherlock-admin3 commented Aug 30, 2024

turvec

Medium

Attackers can cause DOS attacks for tokens with Cap

Summary

Attackers can prevent users from depositing certain tokens into the protocol, causing DOS attacks for tokens with Cap

Vulnerability Detail

Users are able to deposit tokens into the vault using the deposit() function, however, certain tokens have a deposit limit or cap:

    function deposit(ERC20 _token, uint256 _amount, address _receiver) public {
        uint256 cap = caps[address(_token)];

        if (cap != type(uint256).max) {
@>          if (_amount + _token.balanceOf(address(this)) > cap) {
                revert DepositExceedsCap();
            }
        }
         ...
    }

These tokens can also be withdrawn at any time through the withdraw() function with no regulation capabilities whatsoever:

function withdraw(ERC20 _token, uint256 _amount, address _receiver) public {
        balances[msg.sender][_token] -= _amount;

        _token.safeTransfer(_receiver, _amount);

        emit Withdraw(msg.sender, _receiver, address(_token), _amount);
    }

This allows attackers to run bots that monitor the deposit transactions to any of these capped tokens and front-run it to deposit the capped amount and then back-run it to withdraw out the amount, causing users' deposit transactions to those tokens to revert.
There are multiple reasons for attackers to perform such malicious attacks such as competitor advantage or blackhat reasons e.t.c

Impact

Attackers can cause DOS attacks for tokens with Cap

Code Snippet

PointTokenVault.sol#L118

Tool used

Manual Review

Recommendation

In order to prevent these attacks for tokens with caps, consider implementing a minimum cooldown on withdrawals and a regulating capability of penalizing such activities e.g slashing, temporary blocking or seizing of withdrawals for such blacklisted addresses.

@sherlock-admin3 sherlock-admin3 added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Aug 30, 2024
@github-actions github-actions bot closed this as completed Sep 1, 2024
@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label Sep 1, 2024
@Alireza-Razavi
Copy link
Collaborator

This seems to be low due to sponsor's comment:

This is technically accurate, but would make little to no economic sense for an “attacker”. All funds used in the DOS attack would be permenantly lost by the attacker. The cap can extremely easily be raised. It only applies to transactions that would move the balance to exactly or just under the cap, which is presumable a very small subset of deposits into the vault

@sherlock-admin2 sherlock-admin2 changed the title Straight Cotton Tapir - Attackers can cause DOS attacks for tokens with Cap turvec - Attackers can cause DOS attacks for tokens with Cap Sep 3, 2024
@sherlock-admin2 sherlock-admin2 added the Non-Reward This issue will not receive a payout label Sep 3, 2024
@turvec
Copy link

turvec commented Sep 3, 2024

Escalate
The sponsor's comment might have misunderstood some points as these statements above are wrong.
Therefore I will give 2 further explanations:

  1. All funds used in the DOS attack would be permanently lost by the attacker.

As explained in the report above, there is a withdraw() function with no time gap, blacklisting, or any regulation capabilities whatsoever, this allows the attacker to get back the funds used in the front run attack instantly with an immediate back-run.

  1. It only applies to transactions that would move the balance to exactly or just under the cap, which is presumable a very small subset of deposits into the vault

These apply for any user depositing with any amount as long as attackers have the fund required for the cap

@sherlock-admin3
Copy link
Contributor Author

Escalate
The sponsor's comment might have misunderstood some points as these statements above are wrong.
Therefore I will give 2 further explanations:

  1. All funds used in the DOS attack would be permanently lost by the attacker.

As explained in the report above, there is a withdraw() function with no time gap, blacklisting, or any regulation capabilities whatsoever, this allows the attacker to get back the funds used in the front run attack instantly with an immediate back-run.

  1. It only applies to transactions that would move the balance to exactly or just under the cap, which is presumable a very small subset of deposits into the vault

These apply for any user depositing with any amount as long as attackers have the fund required for the cap

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page,
in the Sherlock webapp.

@sherlock-admin2
Copy link
Contributor

The protocol team fixed this issue in the following PRs/commits:
sense-finance/point-tokenization-vault#38

1 similar comment
@sherlock-admin2
Copy link
Contributor

The protocol team fixed this issue in the following PRs/commits:
sense-finance/point-tokenization-vault#38

@sherlock-admin2
Copy link
Contributor

The Lead Senior Watson signed off on the fix.

@sherlock-admin3 sherlock-admin3 added Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed and removed Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed
Projects
None yet
Development

No branches or pull requests

4 participants