ConvexMasterChef: safeRewardTransfer can cause loss of funds

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/ConvexMasterChef.sol#L299-L306

Vulnerability details

Impact

Same as https://github.com/code-423n4/2022-02-concur-findings/issues/244

All calculations are rounded down, since a lack of tokens in the contracts cannot be rounding errors' fault. So the function is redundant. On the other hand, if the contract is undersupplied with cvx tokens, this will cause depositors to be sent less tokens than needed (or none). This is especially unsafe because the tokens that were lacking are not resembled in accountings at all. Thus a depositor may invoke the safeRewardTransfer and not receive tokens they were supposed to.

Proof of Concept

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/ConvexMasterChef.sol#L299-L306

Tools Used

None

Recommended Mitigation Steps

Use usual safeTransfer instead of safeRewardTransfer

[0xMaharishi]

Reward tokens are transferred here before rewards start

[0xMaharishi]

https://github.com/code-423n4/2022-05-aura/pull/6

[dmvt]

I agree with this report. The fallback situation in this function specifically prioritizes loss of funds over bricking the contract, which while laudable, results in what is effectively a silent failure case.