FundsLib Static Analysis Findings

FLB-01S: Illegible Numeric Value Representation

Type	Severity	Location
Code Style		FundsLib.sol:L181

Description:

The linked representation of a numeric literal is sub-optimally represented decreasing the legibility of the codebase.

Example:

Copy
181buyerPayoff = (pot * dispute.buyerPercent) / 10000;

Recommendation:

To properly illustrate the value's purpose, we advise the following guidelines to be followed. For values meant to depict fractions with a base of 1e18, we advise fractions to be utilized directly (i.e. 1e17 becomes 0.1e18) as they are supported. For values meant to represent a percentage base, we advise each value to utilize the underscore (_) separator to discern the percentage decimal (i.e. 10000 becomes 100_00, 300 becomes 3_00 and so on). Finally, for large numeric values we simply advise the underscore character to be utilized again to represent them (i.e. 1000000 becomes 1_000_000).

Alleviation (44009967e4f68092941d841e9e0f5dd2bb31bf0b):

The Boson Protocol team evaluated this exhibit and opted to retain the current representational style in the codebase as they deem an underscore in small numbers such as the value 100_00 as unnecessary. As a result, we consider this exhibit as acknowledged.

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

Type	Severity	Location
Standard Conformity		FundsLib.sol:L228, L269

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 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
227// transfer ERC20 tokens from the caller
228try IERC20(_tokenAddress).transferFrom(EIP712Lib.msgSender(), address(this), _amount) {} catch (
229    bytes memory error
230) {
231    string memory reason = error.length == 0 ? TOKEN_TRANSFER_FAILED : string(error);
232    revert(reason);
233}

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. The safeTransfer / safeTransferFrom invocations can consequently be wrapped in the existing try-catch blocks to properly illustrate custom error reasons in such cases.

Alleviation (44009967e4f68092941d841e9e0f5dd2bb31bf0b):

Both transfer and transferFrom invocations have been properly replaced by their safe-prefixed counterparts from the OpenZeppelin library alleviating this exhibit in full.