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

closeTrove never nulls trove.stake #581

Closed
code423n4 opened this issue Mar 6, 2023 · 1 comment
Closed

closeTrove never nulls trove.stake #581

code423n4 opened this issue Mar 6, 2023 · 1 comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Core/contracts/TroveManager.sol#L1278-L1293

Vulnerability details

// Auditor's note: not 100% sure if this is intentional, but I have reason to believe it's a mistake.

Description

When a trove gets liquidated, its stake gets set to 0 (through _removeStake, called eg here). However, when a trove gets closed gratiously through _closeTrove, the stake is never nulled.

Exploit scenario

Not nulling a stake could cause unforseen consequences if other areas of the code assume that a non-existent (closed) trove doesn't have a stake.

Recommendation

Either ensure all data relating to a trove gets nulled on closure, or provide documentation (code comments) for why th following invariant doesn't hold:

  • opening a trove and closing it leaves the system unchanged (except for the order of elements in TroveOwners).
@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Mar 6, 2023
code423n4 added a commit that referenced this issue Mar 6, 2023
@c4-judge c4-judge closed this as completed Mar 9, 2023
@c4-judge
Copy link
Contributor

c4-judge commented Mar 9, 2023

trust1995 marked the issue as unsatisfactory:
Out of scope

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants