Omniscia Bonq Audit
stability-pool Static Analysis Findings
stability-pool Static Analysis Findings
STA-01S: Inexistent Sanitization of Input Addresses
Type | Severity | Location |
---|---|---|
Input Sanitization | stability-pool.sol:L115-L119 |
Description:
The linked function(s) accept address
arguments yet do not properly sanitize them.
Impact:
The presence of zero-value addresses, especially in constructor
implementations, can cause the contract to be permanently inoperable. These checks are advised as zero-value inputs are a common side-effect of off-chain software related bugs.
Example:
115constructor(address _factory, address _bonqToken) {116 factory = ITroveFactory(_factory);117 stableCoin = IMintableToken(address(factory.stableCoin()));118 bonqToken = IERC20(_bonqToken);119}
Recommendation:
We advise some basic sanitization to be put in place by ensuring that each address
specified is non-zero.
Alleviation:
The Bonq Protocol team has introduced sufficient checks validating that the input addresses cannot be zero.
STA-02S: Improper Invocations of EIP-20 transfer
/ transferFrom
Type | Severity | Location |
---|---|---|
Standard Conformity | stability-pool.sol:L127, L163, L193, L713, L724 |
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:
127stableCoin.transferFrom(msg.sender, address(this), _amount);
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 the usage of OpenZeppelin's SafeERC20 library for collateralToken
invocations but not for bonqToken
and stableCoin
.