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

Return Value is Not Validated #24

Open
code423n4 opened this issue Aug 22, 2021 · 2 comments
Open

Return Value is Not Validated #24

code423n4 opened this issue Aug 22, 2021 · 2 comments
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

The circuitBreaker() function in RCMarket.sol is utilised in the event an oracle never provides a response to a RealityCards question. The function makes an external call to the RCOrderbook.sol contract through the closeMarket() function. If for some reason the orderbook was unable to be closed, this would never be checked in the circuitBreaker() function.

Proof of Concept

https://github.com/code-423n4/2021-08-realitycards/blob/main/contracts/RCMarket.sol#L1215-L1223

Tools Used

Manual code review

Recommended Mitigation Steps

Ensure this is intended behaviour, or otherwise validate the response of orderbook.closeMarket(). Another option would be to emit the result of the external call in the LogStateChange event, alongside the state change.

@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
Copy link
Collaborator

Splidge commented Aug 26, 2021

We have decided that given the extra complexity added to the contracts since the circuitBreaker was first implemented it will no longer be able to perform all the required functions and so it will be removed.
If this call to the orderbook fails (or is removed) then users would not have their rentalRates reduced and so their funds would slowly be drained until the foreclose.
We also have setAmicableResolution which can be used in the event of an oracle failure.

@Splidge
Copy link
Collaborator

Splidge commented Sep 7, 2021

circuitBreaker removed here

@Splidge Splidge added the Resolved Used when a fix has been implemented. label Sep 7, 2021
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