code-423n4 / 2023-11-shellprotocol-findings

7 stars 7 forks source link

Can sent interaction with more than userBalance, might result in wrong accounting in Ocean protocol #208

Closed c4-bot-5 closed 10 months ago

c4-bot-5 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/ocean/Ocean.sol#L522-L524 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/ocean/Ocean.sol#L535

Vulnerability details

Impact

In the Ocean::doMultipleInteraction the WrapErc20 interaction type uses the specifiedAmount to set the user balance inside Ocean. A user is able to set a specified amount higher than the user token balance.

It doesn't revert, because the token decimals are converted and the amount is properly minted. Still balanceDeltas are set to a higher balance than the user originally has.

However I couldn't find a way to potentially exploit it, as an Accounting protocol it feels wrong not validating the user balance.

Proof of Concept

1. Fetch the current inputAddress token balance of wallet 
2. Create a new interaction with type WrapErc20
2.1 SpecifiedAmount can be set to walletBalance * 1e6
3. call doMultipleInteractions as intended

Tools Used

Foundry

Recommended Mitigation Steps

Validate the user balance in the interactions loop

if (IERC20(externalContract).balanceOf(msg.sender) < interaction.specifiedAmount) {
    revert("Ocean: insufficient balance");
}

Assessed type

Invalid Validation

c4-pre-sort commented 10 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #14

c4-judge commented 10 months ago

0xA5DF marked the issue as unsatisfactory: Invalid