BONQ-staking Static Analysis Findings

BOQ-01S: Improper Invocations of EIP-20 transfer / transferFrom

Type	Severity	Location
Standard Conformity		BONQ-staking.sol:L148, L187, L200

Description:

The linked statements do not properly validate the returned bool values of the EIP-20 standard transfer & transferFrom functions. As the standard dictates, callers must not assume that false is never returned.

Impact:

If the code mandates that the returned bool is true, this will cause incompatibility with tokens such as USDT / Tether as no such bool is returned to be evaluated causing the check to fail at all times. On the other hand, if the token utilized can return a false value under certain conditions but the code does not validate it, the contract itself can be compromised as having received / sent funds that it never did.

Example:

Copy
148bonqToken.transferFrom(msg.sender, address(this), _bonqAmount);

Recommendation:

Since not all standardized tokens are EIP-20 compliant (such as Tether / USDT), we advise a safe wrapper library to be utilized instead such as SafeERC20 by OpenZeppelin to opportunistically validate the returned bool only if it exists in each instance.

Alleviation:

The Bonq Protocol team has introduced a require check validating that the result of the bonqToken.transferFrom function is true, however, the bonqToken implementation is not known and as such may not yield true on its transferFrom invocation. We advise a safe wrapper library to be utilized instead to ensure that the yielded bool is evaluated opportunistically (i.e. only if it exists). We also advised to evaluate the bool for bonqToken.transfer and stableCoin.transferFrom statements but the bool value has not been evaluated at these statements yet.