search

code-423n4 / 2022-02-concur-findings

2 stars 0 forks source link

Transfer to treasury can register as succeeded when failing in `_calcRewardIntegral` #120

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-concur/blob/72b5216bfeaa7c52983060ebfc56e72e0aa8e3b0/contracts/StakingRewards.sol#L126

Vulnerability details

Impact

If the transfer of the reward token fails to the treasury (due to insufficient funds for example), the function _calcRewardIntegral will still update accounting and cause devastating accounting discrepancies in the contract.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Tools Used

Recommended Mitigation Steps

require(IERC20(reward.token).transfer(treasury, d_reward / 5), "ERROR_MESSAGE");

r2moon commented 2 years ago

https://github.com/code-423n4/2022-02-concur-findings/issues/263

GalloDaSballo commented 2 years ago

All in all this report is the classic "No safeApprove". But with an actual idea of a POC Ultimately the risk is contingent on the specific reward.token being a nonRevertingOnError

For that reason, I believe Medium Severity to be more appropriate