search

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

2 stars 0 forks source link

Re-entrancy vulnerabilities #249

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-concur/blob/main/contracts/ConcurRewardPool.sol#L34-L40

Vulnerability details

Impact

Function claimRewards in ConcurRewardPool should be re-entrancy protected or first nullify the reward before sending it, otherwise, if any token contains a transfer callback hook, users can claim the same rewards multiple times, by re-entering the function upon the transfer and repeating the call.

Also, consider applying re-entrancy protection to functions that do not follow the Check-Effects-Interaction pattern, even though they are interating with tokens that are supposed to be safe. For instance, functions deposit and withdraw in USDMPegRecovery first send the tokens and only then set the state.

Recommended Mitigation Steps

Solution: either apply nonReentrant modifier or substitute these lines in opposite order:

  IERC20(_tokens[i]).safeTransfer(msg.sender, getting);
  reward[msg.sender][_tokens[i]] = 0;
r2moon commented 2 years ago

duplicated with https://github.com/code-423n4/2022-02-concur-findings/issues/260

GalloDaSballo commented 2 years ago

Dup of #86