No emmission of events after sensitive action in RCNftHub2.sol and in RCtreasury.sol

[code423n4]

Handle

JMukesh

Vulnerability details

Impact

no emission of events in deposit() and withdrwal() function in RCNftHub2.sol
no emssion of events in topupMarketBalance() , payRent(), payout(), refund() in RCtreasury.sol

events are helpful to track off-chain changes

Proof of Concept

https://github.com/code-423n4/2021-06-realitycards/blob/main/contracts/nfthubs/RCNftHubL2.sol#L135

https://github.com/code-423n4/2021-06-realitycards/blob/main/contracts/nfthubs/RCNftHubL2.sol#L157

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

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

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

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

Tools Used

Recommended Mitigation Steps

[Splidge]

no emission of events in deposit() and withdrwal() function in RCNftHub2.sol

deposit() and withdraw() both emit standard ERC721 Transfer events via the respective _mint and _burn calls.

no emssion of events in topupMarketBalance() , payRent(), payout(), refund() in RCtreasury.sol

• topupMarketBalance() will probably only be used once as it's estimated that 1xDai could prevent rounding issues for a decade or more, regardless it does emit an ERC20 Transfer event.
• payRent() emits LogRentCollection from the market that calls it, this makes it nice and clear which market collected the rent
• payout() emits a variety of events based on why the payout was given, again from the market that calls it:
  • LogRentReturned in the event of an invalid outcome,
  • LogWinningsPaid for winnings from the prize pot and
  • LogStakeholderPaid for the market creator, affiliate, card affiliates and artist
• refund(), there is no refund() function but there is a refundUser() which does emit LogAdjustDeposit, I assume this is the one you mean as there's only 6 instances of the word "refund" in the Treasury and they're all in refundUser()

Note that payRent(), payout() and refundUser() all have the onlyMarkets modifier so we can be sure that the markets are the only contracts calling these functions and so the events will always be sent.

[dmvt]

I agree with the sponsor on this one