code-423n4 / 2021-09-swivel-findings

0 stars 0 forks source link

exitZcTokenFillingZcTokenInitiate in Swivel.sol, token transfer may fail without function reverting #127

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

GalloDaSballo

Vulnerability details

Impact

exitZcTokenFillingZcTokenInitiate in Swivel.sol https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/swivel/Swivel.sol#L241

uses transferFrom this function can fail(meaning tokens are not transferred),without causing a revert. This can break the accounting of the protocol

The reason why this can happen is that certain ERC20s will return false on failure instead of reverting. If you don't check for the return value, then you have no guarantee that the transfer was successful (unless you check balance change or if you just use safeTransferFrom by OpenZeppelin

Proof of Concept

See a similar issue in Pool Together's contest: https://github.com/code-423n4/2021-07-pooltogether-findings/issues/61

Recommended Mitigation Steps

Replace transferFrom with OpenZeppelin's safeTransferFrom