sherlock-audit / 2024-05-sophon-judging

7 stars 6 forks source link

Avci - the `SophonFarming.sol` contract assumes all tokens in the contract have 18 decimals #202

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 4 months ago

Avci

medium

the SophonFarming.sol contract assumes all tokens in the contract have 18 decimals

Summary

the SophonFarming.sol contract assumes all tokens in the contract have 18 decimals but we have add() function in the contract to add new pools and new tokens to contract.

Vulnerability Detail

all of the point calculations in the SophonFarming.sol contract in functions like _pendingPoints(), _deposit(), increaseBoost(), and withdraw() assume all tokens have 18 decimal. all tokens in the initialize() function have 18 decimals but we have an add() function to add more pool. As I asked the sponsor they may add tokens that don't have 18 decimals like USDC and it makes all calculations wrong in the system and can lead to wrong reward distribution.

Impact

this issue can lead to unfair reward distribution and loss of points for some users.

Code Snippet

for exmaple lets check _pendingPoints() function.

function _pendingPoints(uint256 _pid, address _user) internal view returns (uint256) {
        PoolInfo storage pool = poolInfo[_pid];
        UserInfo storage user = userInfo[_pid][_user];

        uint256 accPointsPerShare = pool.accPointsPerShare * 1e18;

        uint256 lpSupply = pool.amount;
        if (getBlockNumber() > pool.lastRewardBlock && lpSupply != 0) {
            uint256 blockMultiplier = _getBlockMultiplier(pool.lastRewardBlock, getBlockNumber());

            uint256 pointReward =
                blockMultiplier *
                pointsPerBlock *
                pool.allocPoint /
                totalAllocPoint;

            accPointsPerShare = pointReward *
                1e18 /
                lpSupply +
                accPointsPerShare;
        }

        return user.amount *
            accPointsPerShare /
            1e36 +
            user.rewardSettled -
            user.rewardDebt;
    }

https://github.com/sherlock-audit/2024-05-sophon/blob/05059e53755f24ae9e3a3bb2996de15df0289a6c/farming-contracts/contracts/farm/SophonFarming.sol#L357-L384

Tool used

Manual Review

Recommendation

consider checking decimal() dynamically to support tokens with different decimals than 18.

sherlock-admin4 commented 4 months ago

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

0xmystery commented:

invalid because pending points are individually pid linked. Normalization outside the contract can always be worked out when air dropping