sherlock-audit / 2024-07-sense-points-marketplace-judging

2 stars 0 forks source link

Afriaudit - Arbitrary Deposit into Contract Can Cause DoS to User #180

Closed sherlock-admin4 closed 2 weeks ago

sherlock-admin4 commented 2 weeks ago

Afriaudit

Medium

Arbitrary Deposit into Contract Can Cause DoS to User

Summary

The PointTokenVault contract is vulnerable to a Denial of Service (DoS) condition due to its method of checking deposit limits in function deposit .

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

The contract uses the current token balance to enforce deposit caps, which inadvertently includes arbitrary deposits not accounted for by the intended deposit tracking mechanism, potentially causing legitimate deposit transactions to revert before the total intended deposits reach the cap.

Vulnerability Detail

The contract enforces deposit caps by comparing the sum of the deposit amount and the contract's current balance. This approach inadvertently includes any arbitrary deposits made to the contract, reducing the effective cap limit set by the admin. As a result, legitimate deposit attempts may revert when the actual tracked deposits have not yet reached the cap, due to these untracked arbitrary deposits.

Impact

https://github.com/sherlock-audit/2024-07-sense-points-marketplace/blob/main/point-tokenization-vault/contracts/PointTokenVault.sol#L118

Code Snippet

Tool used

Arbitrary deposits can prematurely hit the cap, blocking legitimate user deposits.

Manual Review

Recommendation

Track total deposits separately from the contract's balance

function deposit(ERC20 _token, uint256 _amount, address _receiver) public {
        uint256 cap = caps[address(_token)];
// This will tract total deposited token instead of token.balanceOf(address(this)) 

      +  uint256 currentTotalDeposits = totalDeposits[address(_token)];

        if (cap != type(uint256).max) {
            if (currentTotalDeposits + _amount > cap) {
                revert DepositExceedsCap();
            }
        }

        _token.safeTransferFrom(msg.sender, address(this), _amount);

        balances[_receiver][_token] += _amount;
    +    totalDeposits[address(_token)] += _amount;

    }
}

So only successful deposits should count towards the cap.