sherlock-audit / 2024-03-zap-protocol-judging

3 stars 1 forks source link

ZdravkoHr. - Users able to deposit over maxAllocation because of wrong implementation of `TokenSale.calculateMaxAllocation()` #167

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

ZdravkoHr.

high

Users able to deposit over maxAllocation because of wrong implementation of TokenSale.calculateMaxAllocation()

Summary

Instead of using the maxAllocation as a ceiling for deposits, its used as a floor.

Vulnerability Detail

TokenSale.calculateMaxAllocation() will return $max(userAllocation, maxAllocation)$. This means that users can deposit over the maxAllocation limit.

Impact

Breaks a core invariant of the protocol and leads to unfair distribution.

Code Snippet

    function calculateMaxAllocation(address _sender) public returns (uint256) {
        uint256 userMaxAllc = _maxTierAllc(_sender);

        if (userMaxAllc > maxAllocation) {
            return userMaxAllc;
        } else {
            return maxAllocation;
        }
    }

Tool used

Manual Review

Recommendation

    function calculateMaxAllocation(address _sender) public returns (uint256) {
        uint256 userMaxAllc = _maxTierAllc(_sender);

-        if (userMaxAllc > maxAllocation) {
+       if (userMaxAllc < maxAllocation) {
            return userMaxAllc;
        } else {
            return maxAllocation;
        }
    }