code-423n4 / 2021-07-spartan-findings

0 stars 0 forks source link

Router.removeLiquiditySingle(uint256,bool,address) has unchecked transfers #189

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

heiho1

Vulnerability details

Impact

Router.removeLiquiditySingle(uint256,bool,address) on lines 121, 126, 129 ignores the boolean return on transfers. This is a brittle implementation because it relies on the boolean return value being hard-coded to true. Tokens may return false instead of reverting and this could potentially lead to more removal than permitted.

Proof of Concept

https://github.com/code-423n4/2021-07-spartan/blob/e2555aab44d9760fdd640df9095b7235b70f035e/contracts/Router.sol#L121

Tools Used

Slither

Recommended Mitigation Steps

There is no particular disadvantage to a require(success, "!transfer") check

SamusElderg commented 3 years ago

Duplicate of #8

ghoul-sol commented 3 years ago

https://github.com/code-423n4/2021-07-spartan-findings/issues/8#issuecomment-894852860