QA Report

[code423n4]

Rounding issue in ConvexStakingWrapper

rouding issue ConvexStakingWrapper might lead to revert
https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/ConvexStakingWrapper.sol#L178

        if (reward.token == cvx || reward.token == crv) {
            IERC20(reward.token).transfer(treasury, d_reward / 5);
            d_reward = (d_reward * 4) / 5;
        }
        IERC20(reward.token).transfer(address(claimContract), d_reward);

fix by changing to

        if (reward.token == cvx || reward.token == crv) {
            IERC20(reward.token).transfer(treasury, d_reward / 5);
            d_reward = d_reward - d_reward / 5;
        }
        IERC20(reward.token).transfer(address(claimContract), d_reward);

Lack input validation in constructor of ConvexStakingWrapper

masterChef can be set to address(0) which will require redeployment
https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/ConvexStakingWrapper.sol#L71

Cast to uint192 is unsafe

Cast to uint192 is unsafe, user trying to deposit more than 2^192 would recevie less deposits balance.
https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/ConvexStakingWrapper.sol#L235

        deposits[_pid][msg.sender].amount += uint192(_amount);

Lack input validation in constructor of ConcurRewardPool

rewardNotifier can be set to address(0) which will require redeployment
https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/ConcurRewardPool.sol#L15

USDMPegRecovery withdraw does not check if user have sufficient balance

USDMPegRecovery withdraw does not check if user have sufficient balance. While it would revert later in the procedure due to an overflow when deducting from user.usdm/user.pool3, it is better to have the check at the begining.
https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/USDMPegRecovery.sol#L110

    function withdraw(Liquidity calldata _withdrawal) external {
        Liquidity memory total = totalLiquidity;
        Liquidity memory user = userLiquidity[msg.sender];
        if(_withdrawal.usdm > 0) {
            require(unlockable, "!unlock usdm");
            usdm.safeTransfer(msg.sender, uint256(_withdrawal.usdm));
            total.usdm -= _withdrawal.usdm;
            user.usdm -= _withdrawal.usdm;
        }

        if(_withdrawal.pool3 > 0) {
            pool3.safeTransfer(msg.sender, uint256(_withdrawal.pool3));
            total.pool3 -= _withdrawal.pool3;
            user.pool3 -= _withdrawal.pool3;
        }
        totalLiquidity = total;
        userLiquidity[msg.sender] = user;
        emit Withdraw(msg.sender, _withdrawal);
    }

Use struct reference instead of memory copy

A lot of pattern in the code is like

RewardType memory reward = rewards[_pid][_index];
// do something to reward
rewards[_pid][_index] = reward;

it might be better to use

RewardType storage reward = rewards[_pid][_index];
// do something to reward

gas-wise they should be the same, but using reference make it less likely to forget commit back to storage.

Typo

RADSs should be CONCURs
https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/MasterChef.sol#L25

    // We do some fancy math here. Basically, any point in time, the amount of RADSs

No need to use SafeMath in Solidity >=0.8.0

Math is safe by default in Solidity >=0.8.0
https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/MasterChef.sol#L14

    using SafeMath for uint;

Also need to replace SafeMath function to normal arithmetic in the MasterChef contract.

[GalloDaSballo]

Rounding issue in ConvexStakingWrapper
Agree with rounding, don't believe any revert will happen

Lack input validation in constructor of ConvexStakingWrapper
Agree

Cast to uint192 is unsafe
Agree, dup of #194 (med)

Lack input validation in constructor of ConcurRewardPool
Agree

USDMPegRecovery withdraw does not check if user have sufficient balance
Disagree to fail early as it would cost more gas in the normal case

Use struct reference instead of memory copy
Disagree

No need to use SafeMath in Solidity >=0.8.0
Gas at best

[GalloDaSballo]

3

[JeeberC4]

@CloudEllie please create new issue for the Med finding above.

[CloudEllie]

Created a separate issue for "Cast to uint192 is unsafe" - see #271