Router.swapSynthToAsset(uint256,address,address) on line 249 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 and not revert in which case unexptected swappage may result.
Handle
heiho1
Vulnerability details
Impact
Router.swapSynthToAsset(uint256,address,address) on line 249 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 and not revert in which case unexptected swappage may result.
Proof of Concept
https://github.com/code-423n4/2021-07-spartan/blob/e2555aab44d9760fdd640df9095b7235b70f035e/contracts/Router.sol#L249
Tools Used
Slither
Recommended Mitigation Steps
There is no particular disadvantage to a
require(success, "!transfer")
check