code-423n4 / 2022-03-volt-findings

0 stars 0 forks source link

Code credits fee-on-transfer tokens for amount stated, not amount transferred #100

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-volt/blob/f1210bf3151095e4d371c9e9d7682d9031860bbd/contracts/peg/NonCustodialPSM.sol#L274-L284

Vulnerability details

Impact

The code miscalculates the amountVoltOut because it relies on the value of amountIn rather than the actual amount transferred in the call to safeTransferFrom(). The calculation should take into account the fees charged as a part of the transfer so that the protocol does not leak value.

Proof of Concept

        amountVoltOut = _getMintAmountOut(amountIn);
        require(
            amountVoltOut >= minVoltAmountOut,
            "PegStabilityModule: Mint not enough out"
        );

        underlyingToken.safeTransferFrom(
            msg.sender,
            address(pcvDeposit),
            amountIn
        );

https://github.com/code-423n4/2022-03-volt/blob/f1210bf3151095e4d371c9e9d7682d9031860bbd/contracts/peg/NonCustodialPSM.sol#L274-L284

Tools Used

Code inspection

Recommended Mitigation Steps

Measure the balance before and after the call to safeTransferFrom(), and use the difference between the two as the amount, rather than amountIn

ElliotFriedman commented 2 years ago

PSM will only use tokens that don't have fee-on-transfer logic or have that enabled.

ElliotFriedman commented 2 years ago

duplicate https://github.com/code-423n4/2022-03-volt-findings/issues/21