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

Stader OPERATOR is single point of failure #344

Open
code423n4 opened this issue Jun 9, 2023 · 6 comments
Open

Stader OPERATOR is single point of failure #344

code423n4 opened this issue Jun 9, 2023 · 6 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 downgraded by judge Judge downgraded the risk level of this issue M-03 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/2023-06-stader/blob/main/contracts/PermissionlessNodeRegistry.sol#L183
https://github.com/code-423n4/2023-06-stader/blob/main/contracts/PermissionedNodeRegistry.sol#L254

Vulnerability details

Impact

The OPERATOR role holds a lot of power within the system, which can compromise the both the system integrity and it's permission-less nature.

Proof of Concept

The OPERATOR key is responsible for confirming marking each validator submitted key as either valid or invalid, without any assurance to validators.

  1. Arbitrary negation of participation makes permissionless pool permissioned.
    The documentation states:

Any validator in permissionless pool can run a node with 4 ETH + 0.4 ETH worth of SD token.

Which is not strictly true, since any participant in the system must be vetted by the OPERATOR, which can arbitrarily mark as invalid or frontrun key without the need to provide justification or having an appeal system. Alternatively, the OPERATOR can simple ignore the added key and never mark it as ready to deposit.

Therefore, the pool can't be considered permissionless, since participants must rely on the benevolence of the OPERATOR to participate.

  1. Authorization of invalid keys
    There is no way for the smart contract system to check or confirm that a given public key is really legit, and could generate income to ETHx holders, so the system relies solely on the OPERATOR to make that distinction, rendering the system vulnerable in case of a comprised wallet.

Tools Used

Manual Review

Recommended Mitigation Steps

There is no simple fix for the issue, but at minimum, the protocol shouldn't be advertised as permissioneless.

Assessed type

Rug-Pull

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

Picodes changed the severity to 2 (Med Risk)

@manoj9april
Copy link

Thank you pointing it out.
We will move this logic to oracle.

@c4-sponsor
Copy link

manoj9april 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 Jun 20, 2023
@Picodes
Copy link

Picodes commented Jul 2, 2023

Keeping Med severity considering this could be an instance of "function of the protocol or its availability could be impacted"

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jul 2, 2023
@c4-judge
Copy link
Contributor

c4-judge commented Jul 2, 2023

Picodes marked the issue as satisfactory

@C4-Staff C4-Staff added selected for report This submission will be included/highlighted in the audit report M-03 labels Jul 7, 2023
@sanjay-staderlabs
Copy link

This is fixed

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 downgraded by judge Judge downgraded the risk level of this issue M-03 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

7 participants