QA Report

[code423n4]

Title: Not verified input Severity: Low Risk

external / public functions parameters should be validated to make sure the address is not 0.
Otherwise if not given the right input it can mistakenly lead to loss of user funds.

    RewardDistributor.sol._claim _account
    ThecosomataETH.sol.withdraw recipient

Title: Treasury may be address(0) Severity: Low Risk

Make sure the treasury is not address(0).

    ThecosomataETH.sol.constructor _TREASURY

Title: safeApprove of openZeppelin is deprecated Severity: Low Risk

    You use safeApprove of openZeppelin although it's deprecated.
    (see https://github.com/OpenZeppelin/openzeppelin-contracts/blob/566a774222707e424896c0c390a84dc3c13bdcb2/contracts/token/ERC20/utils/SafeERC20.sol#L38)
    You should change it to increase/decrease Allowance as OpenZeppilin says
    This appears in the following locations in the code base

    Deprecated safeApprove in ThecosomataETH.sol line 67:         IERC20(_BTRFLY).approve(_CURVEPOOL, 2**256 - 1);

    Deprecated safeApprove in ThecosomataETH.sol line 68:         IERC20(_WETH).approve(_CURVEPOOL, 2**256 - 1);

Title: Missing non reentrancy modifier Severity: Low Risk

The following functions are missing reentrancy modifier although some other pulbic/external functions does use reentrancy modifer. Even though I did not find a way to exploit it, it seems like those functions should have the nonReentrant modifier as the other functions have it as well..

    RewardDistributor.sol, receive is missing a reentrancy modifier
    RewardDistributor.sol, updateRewardsMetadata is missing a reentrancy modifier
    RewardDistributor.sol, setBribeVault is missing a reentrancy modifier

Title: In the following public update functions no value is returned Severity: Non Critical

In the following functions no value is returned, due to which by default value of return will be 0. We assumed that after the update you return the latest new value. (similar issue here: https://github.com/code-423n4/2021-10-badgerdao-findings/issues/85).

    RewardDistributor.sol, updateRewardsMetadata

Title: Named return issue Severity: Low Risk

Users can mistakenly think that the return value is the named return, but it is actually the actualreturn statement that comes after. To know that the user needs to read the code and is confusing. Furthermore, removing either the actual return or the named return will save gas.

    ThecosomataETH.sol, checkUpkeep

Title: Anyone can withdraw others Severity: Low Risk

Anyone can withdraw users shares. Although we think that they are sent to the right address, it is still 1) not the desired behavior 2) can be dangerous if the receiver is a smart contract 3) the receiver may not know someone withdraw him

    ThecosomataETH.withdraw

Title: transfer return value of a general ERC20 is ignored Severity: High Risk

Need to use safeTransfer instead of transfer. As there are popular tokens, such as USDT that transfer/trasnferFrom method doesn’t return anything. The transfer return value has to be checked (as there are some other tokens that returns false instead revert), that means you must

1. Check the transfer return value Another popular possibility is to add a whiteList. Those are the appearances (solidity file, line number, actual line):

   ThecosomataETH.sol, 164 (withdraw), IERC20(token).transfer(recipient, amount);
   ThecosomataETH.sol, 146 (performUpkeep), IERC20(token).transfer(TREASURY, tokenBalance);

[drahrealm]

Duplicate issues for usage of transfer and checkUpkeep. As for withdraw no users fund are at risk since owner can only pull put what is intended to be used by the contract for adding liquidity to the pool

[GalloDaSballo]

Report could have been better formatted with titles and syntax highlighting

Also could have linked to code

Findings are valid

[GalloDaSballo]

3/10

Were there links and proper formatting I'd rate higher