code-423n4 / 2023-05-juicebox-findings

1 stars 1 forks source link

If `transfer()` call failed silently then caller will loss his funds. #227

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-juicebox/blob/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L286

Vulnerability details

Impact

User will loss fund.

Proof of Concept

According to code snippet _nonReservedToken will send to beneficiary via transfer function. Some function doesnot revert on failure, they return false.

In that case Protocol doesn't checking return value from transfer().

// The amount to send to the beneficiary
        uint256 _nonReservedToken = PRBMath.mulDiv(
            _amountReceived, JBConstants.MAX_RESERVED_RATE - _reservedRate, JBConstants.MAX_RESERVED_RATE
        );

        // The amount to add to the reserved token
        uint256 _reservedToken = _amountReceived - _nonReservedToken;

        // Send the non-reserved token to the beneficiary (if any / reserved rate is not max)
        if (_nonReservedToken != 0) projectToken.transfer(_data.beneficiary, _nonReservedToken);

Tools Used

Manual Review

Recommended Mitigation Steps\

Use safeTransfer()

Assessed type

ERC20

dmvt commented 1 year ago

spam / bot race

c4-pre-sort commented 1 year ago

dmvt marked the issue as low quality report

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Out of scope