community-liquidation-pool Static Analysis Findings

COM-01S: Inexistent Sanitization of Input Addresses

Type	Severity	Location
Input Sanitization		community-liquidation-pool.sol:L19-L22

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:

Copy
19constructor(address _factory, address _token) {
20  factory = ITroveFactory(_factory);
21  collateralToken = IERC20(_token);
22}

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 address cannot be zero.

COM-02S: Improper Invocations of EIP-20 transfer / transferFrom

Type	Severity	Location
Standard Conformity		community-liquidation-pool.sol:L47, L63

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
47tokenCache.transfer(msg.sender, _unclaimedCollateral);

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 OpenZeppelin's SafeERC20 library to perform the transfer operation.