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

payout doesn't fix isForeclosed state #28

Open
code423n4 opened this issue Jun 14, 2021 · 2 comments
Open

payout doesn't fix isForeclosed state #28

code423n4 opened this issue Jun 14, 2021 · 2 comments

Comments

@code423n4
Copy link
Contributor

Handle

gpersoon

Vulnerability details

Impact

The function payout of RCTreasury.sol doesn't undo the isForeclosed state of a user.
This would be possible because with a payout a user will receive funds so he can lose his isForeclosed status.

For example the function refundUser does check and update the isForeclosed status.

Proof of Concept

// https://github.com/code-423n4/2021-06-realitycards/blob/main/contracts/RCTreasury.sol#L429
function payout(address _user, uint256 _amount) external override balancedBooks onlyMarkets returns (bool) {
require(!globalPause, "Payouts are disabled");
assert(marketPot[msgSender()] >= _amount);
user[_user].deposit += SafeCast.toUint128(_amount);
marketPot[msgSender()] -= _amount;
totalMarketPots -= _amount;
totalDeposits += _amount;
emit LogAdjustDeposit(_user, _amount, true);
return true;
}

// https://github.com/code-423n4/2021-06-realitycards/blob/main/contracts/RCTreasury.sol#L447
function refundUser(address _user, uint256 _refund) external override onlyMarkets {
...
if ( isForeclosed[_user] && user[_user].deposit > user[_user].bidRate / minRentalDayDivisor ) {
isForeclosed[_user] = false;
emit LogUserForeclosed(_user, false);
}

Tools Used

Recommended Mitigation Steps

Check and update the isForeclosed state in the payout function

@Splidge
Copy link
Collaborator

Splidge commented Jun 14, 2021

The severity of this could be increased as a user might have believed that the payout would cancel their foreclosure.
This could at a push count as 2 (Medium risk) because the "availability could be impacted" as in the definition here. This is because the user wouldn't be allowed to place new bids without calling some other function that will cancel their foreclosure first.

@Splidge
Copy link
Collaborator

Splidge commented Jun 21, 2021

Fixed here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants