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

Stolen rewards in StakingFundsVault #39

Closed
code423n4 opened this issue Nov 14, 2022 · 5 comments
Closed

Stolen rewards in StakingFundsVault #39

code423n4 opened this issue Nov 14, 2022 · 5 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-147 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/StakingFundsVault.sol#L315
https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/StakingFundsVault.sol#L343
https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/SyndicateRewardsProcessor.sol#L67

Vulnerability details

Impact

All rewards can be stolen from StakingFundsVault.

Proof of Concept

https://gist.github.com/clems4ever/b7dd7a6155ac01a9b5e1d8504cd8b5b0

Run with forge test

Tools Used

Manual review and forge

Recommended Mitigation Steps

  • Rework the interaction between accumulatedETHPerLPShare and claimed.
  • Protect the receive function of StakingFundsVault
  • Protect all inhereted but publicly available methods from LPToken.
@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Nov 14, 2022
code423n4 added a commit that referenced this issue Nov 14, 2022
@dmvt
Copy link

dmvt commented Nov 20, 2022

I've asked the warden (in Discord) to add a few more comments to the gist:

"I think this is valid a different from the others you reported, but I'm having a slightly hard time with the use of the mocked privileged user (manager) in the middle of the test. More color to help explain how the hack happens without access to this user is important for validity and risk rating.
Please only add comments, don't remove anything"

@c4-judge c4-judge added the primary issue Highest quality submission among a set of duplicates label Nov 20, 2022
@c4-judge
Copy link
Contributor

dmvt marked the issue as primary issue

@c4-judge
Copy link
Contributor

dmvt marked the issue as duplicate of #59

@c4-judge c4-judge added duplicate-59 and removed primary issue Highest quality submission among a set of duplicates labels Nov 20, 2022
@c4-judge
Copy link
Contributor

dmvt marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Nov 29, 2022
@C4-Staff
Copy link
Contributor

JeeberC4 marked the issue as duplicate of #147

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 duplicate-147 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

4 participants