Elastic-Finance-DAO / eefi_contracts

0 stars 0 forks source link

[DET-01M] Potentially Dangerous Default #91

Open stalker474 opened 2 months ago

stalker474 commented 2 months ago

DET-01M: Potentially Dangerous Default

Type Severity Location
Standard Conformity Distribute.sol:L88

Description:

The default decimal value yielded by the Distribute::tryGetDecimals function in case the call fails is non-standard as most EIP-20 tokens employ 18 decimals.

Impact:

The severity of this exhibit is informational as this value remains unutilized in the Elastic Finance codebase.

Example:

/**
 * @dev Attempts to call the `decimals()` function on an ERC-20 token contract.
 * @param tokenAddress The address of the ERC-20 token contract.
 * @return success Indicates if the call was successful.
 * @return decimals The number of decimals the token uses, or 0 if the call failed.
 */
function tryGetDecimals(address tokenAddress) public view returns (bool success, uint8 decimals) {
    bytes memory payload = abi.encodeWithSignature("decimals()");
    // Low-level call to the token contract
    bytes memory returnData;
    (success, returnData) = tokenAddress.staticcall(payload);

    // If call was successful and returned data is the expected length for uint8
    if (success && returnData.length == 32) {
        // Decode the return data
        decimals = abi.decode(returnData, (uint8));
    } else {
        // Default to 0 decimals if call failed or returned unexpected data
        return (false, 0);
    }
}

Recommendation:

We advise a default value of 18 to be yielded in case of an unsuccessful external call ensuring that the function yields a high-likelihood default value.

stalker474 commented 2 months ago

Agreed we can return 18 instead, yet we also have a "success" flag set to false, so I consider that there is no point in returning a specific number. Let's go with 18 and close this