Tracer contest - Lucius's results

Handle

Lucius

Vulnerability details

Impact

The functions transerFrom()/transer() do not revert on failure and instead simply return false. Without checks on the return values, the transfers could potentially fail silently allowing unexpected issues with certain token pools. E.G. If a user calls deposit() function on a swap pool or insurance pool which uses a non-reverting token, the transferFrom() could fail and the user balance would still be updated resulting in the user getting free tokens. If a user calls the withdraw() function on a swap or insurance pool the same failure could occur resulting in the user not receiving their tokens.

Proof of Concept

Insurance.sol: https://github.com/code-423n4/2021-06-tracer/blob/74e720ee100fd027c592ea44f272231ad4dfa2ab/src/contracts/Insurance.sol#L51

https://github.com/code-423n4/2021-06-tracer/blob/74e720ee100fd027c592ea44f272231ad4dfa2ab/src/contracts/Insurance.sol#L97

TracerPerpetualSwaps.sol: https://github.com/code-423n4/2021-06-tracer/blob/74e720ee100fd027c592ea44f272231ad4dfa2ab/src/contracts/TracerPerpetualSwaps.sol#L151

https://github.com/code-423n4/2021-06-tracer/blob/74e720ee100fd027c592ea44f272231ad4dfa2ab/src/contracts/TracerPerpetualSwaps.sol#L203

https://github.com/code-423n4/2021-06-tracer/blob/74e720ee100fd027c592ea44f272231ad4dfa2ab/src/contracts/TracerPerpetualSwaps.sol#L514

Recommended Mitigation Steps

Use openzeppelins safeERC20 library safeTransfer()/safeTransferFrom() instead to revert on failure. Alternatively, manually use require() to check return value and revert if return value is false.