search

code-423n4 / 2022-07-swivel-findings

0 stars 1 forks source link

Swivel.splitUnderlying() may revert for compound protocol token #181

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/main/Swivel/Swivel.sol#L585 https://github.com/code-423n4/2022-07-swivel/blob/main/Swivel/Swivel.sol#L709

Vulnerability details

Impact

When calling Swivel.splitUnderlying(), if the p enum is Compound protocol, then most times, the return value might be false when ICompound(c).mint(a) returns uint256 value greater than 0. With the return value being false, splitUnderlying() would revert with error 'deposit failed', making the user unable to split Compound token and mint.

Proof of Concept

  1. Alice calls Swivel.splitUnderlying() with the inputs which include compound protocol as p.
  2. deposit() call is attempted in line 585
  3. if (p == uint8(Protocols.Compound)) is satisfied, and ICompound(c).mint(a) returns amount greater than zero
  4. bool false is returned back to line 585, and splitUnderlying() reverts with Exception error 6 - deposit failed.
  5. Alice is unable to split and mint for compound protocol token

Tools Used

Manual review

Recommended Mitigation Steps

The if-statement may need to be changed appropriately.

JTraversa commented 2 years ago

Compound returns 0 on success and other numbers to indicate their error messages

bghughes commented 2 years ago

https://github.com/code-423n4/2022-07-swivel/blob/main/Swivel/Swivel.sol#L709

Verified in the Compound documentation (https://compound.finance/docs/ctokens#mint); therefore, I am marking this as invalid given the sponsor's implementation seems correct and the warden's argument wrong.