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

7 stars 7 forks source link

Unwrapping function emits event with incorrect values. #257

Closed c4-bot-9 closed 9 months ago

c4-bot-9 commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/ocean/Ocean.sol#L983

Vulnerability details

Impact

The _etherUnwrap function in the smart contract does not correctly emit the EtherUnwrap event. The issue arises when calculating and emitting the fee and transfer amounts. The current implementation deducts the fee, transfers the calculated amount, and emits the event with the transferAmount,feeCharged and user address resulting in an inaccurate representation of the unwrapping process since fees decucted is sent as a wrapped token and still unwrapped.

Proof of Concept

  1. Alice wraps 10 Ether using the wrapEther function.
  2. Alice calls the _etherUnwrap function to unWrap the 10shEth
  3. Fee is deducted (lets assume fee ==1shETH) after being calculated and sent to the ocean
  4. When the unwrap event is emitted the total amount 10shEth will be unwrapped instead of the balance since it's the one transferred out from the protocol

Tools Used

Manual

Recommended Mitigation Steps

The function should unwrap only the remaining amount once fee is charged since its the only one that is transferred after unwrapping

Replace this line of code

emit EtherUnwrap(transferAmount, feeCharged, userAddress);

with this one

emit EtherUnwrap(transferAmount, userAddress);

Assessed type

Context

c4-pre-sort commented 9 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 9 months ago

raymondfam marked the issue as primary issue

raymondfam commented 9 months ago

No dust is entailed with the correct event emitted as intended.

c4-judge commented 9 months ago

0xA5DF marked the issue as unsatisfactory: Invalid