code423n4 commented 3 years ago

Handle

0xsanson

Vulnerability details

Impact

In treasury.deposit, an user can re-enter the function if erc20 is a token that hooks the receiver. By re-entering the user can bypass the check (erc20.balanceOf(address(this)) + _amount) <= maxContractBalance, depositing more tokens that the protocol allows.

Proof of Concept

https://github.com/code-423n4/2021-08-realitycards/blob/main/contracts/RCTreasury.sol#L366

Tools Used

editor

Recommended Mitigation Steps

Consider moving this requirement below the safeTransferFrom function.

require(
  (erc20.balanceOf(address(this)) + _amount) <= maxContractBalance,
  "Limit hit"
);

Splidge commented 3 years ago

If the ERC20 token we were using did have this functionality, it is stated that maxContractBalance is a soft check. We are aware that it's possible to exceed this, the easiest method would simply be to sponsor a market where there is no limit.

0xean commented 3 years ago

Based on sponsors comments the limit is a soft check and was expected to be possible to exceed but its unclear if this was based on re-entrancy. Even with the re-entrancy it doesn't seem like there is any substantial risk. Marking as 0 non-critical

code-423n4 / 2021-08-realitycards-findings