Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

External Call Made Before State Change #23

Open
code423n4 opened this issue Aug 22, 2021 · 1 comment
Open

External Call Made Before State Change #23

code423n4 opened this issue Aug 22, 2021 · 1 comment
Labels
1 (Low Risk) bug Something isn't working Resolved Used when a fix has been implemented. sponsor confirmed

Comments

@code423n4
Copy link
Contributor

Handle

leastwood

Vulnerability details

Impact

There are a number of functions in RCTreasury.sol which make external calls to another contract before updating the underlying market balances. More specifically, these affected functions are deposit(), sponsor(), and topupMarketBalance(). As a result, these functions would be prone to reentrancy exploits. However, as safeTransferFrom() operates on a trusted ERC20 token (RealityCard's token), this issue is of low severity.

Proof of Concept

https://github.com/code-423n4/2021-08-realitycards/blob/main/contracts/RCTreasury.sol#L385-L391
https://github.com/code-423n4/2021-08-realitycards/blob/main/contracts/RCTreasury.sol#L561-L563
https://github.com/code-423n4/2021-08-realitycards/blob/main/contracts/RCTreasury.sol#L459-L461

Tools Used

Manual code review

Recommended Mitigation Steps

Modify the aforementioned functions such that all state changes are made before a call to the ERC20 token using the safeTransferFrom() function.

@code423n4 code423n4 added 1 (Low Risk) bug Something isn't working labels Aug 22, 2021
code423n4 added a commit that referenced this issue Aug 22, 2021
@Splidge Splidge added Resolved Used when a fix has been implemented. sponsor confirmed labels Aug 26, 2021
@Splidge
Copy link
Collaborator

Splidge commented Sep 7, 2021

Fixed here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 (Low Risk) bug Something isn't working Resolved Used when a fix has been implemented. sponsor confirmed
Projects
None yet
Development

No branches or pull requests

2 participants