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

Rotating LPTokens to banned BLS public key #64

Open
code423n4 opened this issue Nov 15, 2022 · 4 comments
Open

Rotating LPTokens to banned BLS public key #64

code423n4 opened this issue Nov 15, 2022 · 4 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden M-02 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/ETHPoolLPFactory.sol#L76

Vulnerability details

Impact

It is possible to rotate LPTokens to a banned BLS public key. This is not a safe action, because it can result in insolvency of the project (specially if the banned BLS public key was malicious).

Proof of Concept

When a user deposits ETH for staking by calling depositETHForStaking, the manager checks whether the provided BLS public key is banned or not.
require(liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key is banned or not a part of LSD network");
https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/StakingFundsVault.sol#L113

If it is not banned the LPToken related to that BLS public key will be minted to the caller, so the number of LPToken related to that BLS public key will be increased.
https://github.com/code-423n4/2022-11-stakehouse/blob/39a3a84615725b7b2ce296861352117793e4c853/contracts/liquid-staking/ETHPoolLPFactory.sol#L125

If it is banned, it will not be possible to stake to this BLS public key, so the number of LPToken will not be increased. But the issue is that it is still possible to increase the LPToken of this BLS public key through rotating LPToken.

In other words, a malicious user can call rotateLPTokens, so that the _oldLPToken will be migrated to _newLPToken which is equal to the LPToken related to the banned BLS public key.

In summary, the vulnerability is that during rorating LPTokens, it is not checked that the _newLPToken is related to a banned BLS public key or not.

Tools Used

Recommended Mitigation Steps

The following line should be added to function rotateLPTokens(...):
require(liquidStakingNetworkManager.isBLSPublicKeyBanned(blsPublicKeyOfNewKnot ) == false, "BLS public key is banned or not a part of LSD network");
https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/ETHPoolLPFactory.sol#L76

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Nov 15, 2022
code423n4 added a commit that referenced this issue Nov 15, 2022
@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value edited-by-warden and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Nov 16, 2022
@c4-judge
Copy link
Contributor

dmvt marked the issue as primary issue

@c4-sponsor
Copy link

vince0656 marked the issue as sponsor confirmed

@c4-sponsor c4-sponsor added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Nov 28, 2022
@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Nov 30, 2022
@c4-judge
Copy link
Contributor

dmvt marked the issue as satisfactory

@c4-judge
Copy link
Contributor

dmvt marked the issue as selected for report

@c4-judge c4-judge added the selected for report This submission will be included/highlighted in the audit report label Nov 30, 2022
@C4-Staff C4-Staff added the M-02 label Dec 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden M-02 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

4 participants