sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

BLACK-PANDA-REACH - User can mint variable numbers of DSU tokens by paying same amount of USDC #207

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

BLACK-PANDA-REACH

medium

User can mint variable numbers of DSU tokens by paying same amount of USDC

Summary

By providing same amount of USDC, user can mint variable numbers of DSU tokens to a certain limit.

Vulnerability Detail

User can mint variable numbers of DSU tokens by paying same amount of USDC.

https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/perennial-mono/packages/perennial/contracts/multiinvoker/MultiInvoker.sol#L237-L242

function _wrap(address receiver, UFixed18 amount) internal { 
    // Pull USDC from the `msg.sender` 
    USDC.pull(msg.sender, amount, true); 

    _handleWrap(receiver, amount); 
}

https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/root/contracts/token/types/Token6.sol#L137-L139

function pull(Token6 self, address benefactor, UFixed18 amount, bool roundUp) internal {
    IERC20(Token6.unwrap(self)).safeTransferFrom(benefactor, address(this), toTokenAmount(amount, roundUp));
}

https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/root/contracts/token/types/Token6.sol#L213-L215

function toTokenAmount(UFixed18 amount, bool roundUp) private pure returns (uint256) {
    return roundUp ? Math.ceilDiv(UFixed18.unwrap(amount), OFFSET) : UFixed18.unwrap(amount) / OFFSET;
}

https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/perennial-mono/packages/perennial/contracts/multiinvoker/MultiInvoker.sol#L352-L361

function _handleWrap(address receiver, UFixed18 amount) internal { 
     // If the batcher is 0 or  doesn't have enough for this wrap, go directly to the reserve 
     if (address(batcher) == address(0) || amount.gt(DSU.balanceOf(address(batcher)))) { 
         reserve.mint(amount); 
         if (receiver != address(this)) DSU.push(receiver, amount); 
     } else { 
         // Wrap the USDC into DSU and return to the receiver 
         batcher.wrap(amount, receiver); 
     } 
}

If amount=1000000999999999999 or 1000000000000000001, both / any number in between will round up to 1000001 when passed in USDC.pull(msg.sender, amount, true) with roundUp=true but amount is not rounded up.

It will pull 1000001 USDC from msg.sender, then in _handleWrap()reserve.mint function will mint the amount (not rounded up) number of DSU tokens. Thus no matter if amount is 1000000999999999999 or 1000000000000000001, msg.sender will always pay 1000001 USDC and mint variable number of DSU tokens i.e., between 1000000000000000001 and 1000000999999999999.

Impact

Users will get less DSU tokens for the same amount of USDC if the amount provided has non-zero digits in any of the last 12 decimal places.

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/perennial-mono/packages/perennial/contracts/multiinvoker/MultiInvoker.sol#L237-L242 https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/perennial-mono/packages/perennial/contracts/multiinvoker/MultiInvoker.sol#L352-L361 https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/root/contracts/token/types/Token6.sol#L137-L139 https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/root/contracts/token/types/Token6.sol#L213-L215

https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/perennial-mono/packages/perennial/contracts/multiinvoker/MultiInvoker.sol#L262-L266

https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/perennial-mono/packages/perennial/contracts/multiinvoker/MultiInvoker.sol#L334-L338

Tool used

Manual Review

Recommendation

Update amount before passing it to the pull(), such that amount = Math.ceilDiv(UFixed18.unwrap(amount), 1e12) * 1e12.

KenzoAgada commented 1 year ago

While valid, this is a loss of $0.000001 for the user. I consider this a valid low issue but not enough to qualify for a medium.