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

0 stars 0 forks source link

Return values of `BEP20.transfer` and `BEP20.transferFrom` are unchecked #233

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

shw

Vulnerability details

Impact

The return values of BEP20.transfer and BEP20.transferFrom are not checked to be true in multiple contracts. The return value could be false if the transferred token is not BEP20-compliant, indicating that the transfer fails, while the calling contract will not notice the failure. As written in the BEP20 specification:

Callers MUST handle false from returns (bool success). Callers MUST NOT assume that false is never returned!

Proof of Concept

The return value check should be applied, especially when the transferred token is not created by the Spartan protocol (and thus could be non-compliant to BEP20):

Unchecked transfer: Pool.sol#L199 Pool.sol#L224 Router.sol#L67 Router.sol#L126 Router.sol#L221

Unchecked transferFrom: poolFactory.sol#L112 Router.sol#L207 Synth.sol#L204 Dao.sol#L266

Recommended Mitigation Steps

Use the SafeBEP20 implementation from PancakeSwap (or SafeERC20 from OpenZeppelin) and call safeTransfer or safeTransferFrom when transferring BEP20 tokens.

SamusElderg commented 3 years ago

Duplicate of #8