code-423n4 / 2024-01-salty-findings

5 stars 3 forks source link

lack of scaling decimal places of erc20 leads to break the protocol invariant #920

Closed c4-bot-5 closed 5 months ago

c4-bot-5 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L140-L165

Vulnerability details

Impact

Protocol didn't scale the erc20 tokens .This will cause protocol will not work as intended.

Proof of Concept

Erc20 tokens have different decimals .Protocol gonna use erc20 which has always not 1e18 .Let's assume user add tokenA and token B as liquidity .Token A has 1e18 decimal places and Token B has 1e6 decimal places.When user add liquidity , there will be lossing funds cause of rounding error due to different decimal places.

uint256 proportionalB = ( maxAmount0 * reserve1 ) / reserve0;

    // proportionalB too large for the specified maxAmountB?
    if ( proportionalB > maxAmount1 )
        {
        // Use maxAmountB and a proportional amount for tokenA instead
    ---->>  addedAmount0 = ( maxAmount1 * reserve0 ) / reserve1;
        addedAmount1 = maxAmount1;
        }
    else
        {
        addedAmount0 = maxAmount0;
        addedAmount1 = proportionalB;
        }

    // Update the reserves
    reserves.reserve0 += uint128(addedAmount0);
    reserves.reserve1 += uint128(addedAmount1);

Tools Used

manual view

Recommended Mitigation Steps

scale all erc20 tokens to 1e18

Assessed type

Invalid Validation

c4-judge commented 5 months ago

Picodes marked the issue as unsatisfactory: Insufficient proof

irving4444 commented 4 months ago

@othernet-global , @Picodes could u guys pls check this report again? Protocol is going to use erc20 which have different decimal places and It didn't scale at all . When calculate for addedLiquidity , there will always be incorrect value . eg, token A is usdt which have 6 decimal places and token B is dai which have 18 deicmals . When calculate for _addliquidity , this will always get unintented price , In worst case , there will be zero cause of rounding error

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L90-L135