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

Lack of Functionality for Signer to Revoke Delegation #11

Closed
c4-bot-4 opened this issue Nov 11, 2024 · 2 comments
Closed

Lack of Functionality for Signer to Revoke Delegation #11

c4-bot-4 opened this issue Nov 11, 2024 · 2 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 🤖_primary AI based primary recommendation 🤖_05_group AI based duplicate group recommendation unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-bot-4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-11-ethena-labs/blob/main/contracts/ustb/UStbMinting.sol#L311
https://github.com/code-423n4/2024-11-ethena-labs/blob/main/contracts/ustb/UStbMinting.sol#L317
https://github.com/code-423n4/2024-11-ethena-labs/blob/main/contracts/ustb/UStbMinting.sol#L326

Vulnerability details

Impact

  • Insufficient Control for Signers: Once a delegation is confirmed, the signer has no method to revoke their delegation. This lack of control can lead to prolonged exposure to potential risks if the signer no longer trusts the benefactor or if circumstances change.
  • Security and Trust Issues: Without the ability to revoke delegation, the security model is weakened, as the signer remains perpetually linked to the benefactor until the benefactor decides to revoke the delegation.
  • Operational and Governance Risks: This limitation could deter signers from delegating in the first place, impacting the contract’s utility and trustworthiness. Users might be wary of delegating if they have no straightforward way to retract that delegation.

Proof of Concept

Root Cause:
The UStbMinting contract implements a system for delegating signing rights through the delegatedSigner mapping. This system enables smart contracts to delegate signing to other address (signer) and allows signer to accept. However, there is no mechanism for the original signer to revoke or reject a delegation once he has accepted.

Relevant Code:

// Mapping to track delegated signer status
mapping(address => mapping(address => DelegatedSignerStatus)) public delegatedSigner;

// Existing functions
function setDelegatedSigner(address _delegateTo) external {
    delegatedSigner[_delegateTo][msg.sender] = DelegatedSignerStatus.PENDING;
    emit DelegatedSignerInitiated(_delegateTo, msg.sender);
}

function confirmDelegatedSigner(address _delegatedBy) external {
    if (delegatedSigner[msg.sender][_delegatedBy] != DelegatedSignerStatus.PENDING) {
        revert DelegationNotInitiated();
    }
    delegatedSigner[msg.sender][_delegatedBy] = DelegatedSignerStatus.ACCEPTED;
    emit DelegatedSignerAdded(msg.sender, _delegatedBy);
}

function removeDelegatedSigner(address _removedSigner) external {
    delegatedSigner[_removedSigner][msg.sender] = DelegatedSignerStatus.REJECTED;
    emit DelegatedSignerRemoved(_removedSigner, msg.sender);
}

Problem Analysis:

  • The removeDelegatedSigner function only allows the benefactor to revoke the delegation, but the original signer has no equivalent function to do so.
  • Scenario:
    • EOA (address A) sets delegate address B(signer) for signing.
    • address B confirms the delegation.
    • address B decides to remove signing rights for address A, but there is no function available for address B to revoke this delegation independently.

Recommended Mitigation Steps

Introduce a Function for Signer-Driven Revocation:
Add a function that allows the original signer to revoke the delegation status:

function revokeDelegatedSigner(address _delegatedBy) external {
    if (delegatedSigner[msg.sender][_delegatedBy] != DelegatedSignerStatus.ACCEPTED) {
        revert DelegationNotAccepted();
    }
    delegatedSigner[msg.sender][_delegatedBy] = DelegatedSignerStatus.REJECTED;
    emit DelegatedSignerRemoved(_delegateTo, msg.sender);
}
@c4-bot-4 c4-bot-4 added 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 labels Nov 11, 2024
c4-bot-6 added a commit that referenced this issue Nov 11, 2024
@c4-bot-11 c4-bot-11 added 🤖_05_group AI based duplicate group recommendation 🤖_primary AI based primary recommendation labels Nov 11, 2024
@c4-judge
Copy link

0xEVom marked the issue as unsatisfactory:
Invalid

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Nov 12, 2024
@0xEVom
Copy link

0xEVom commented Nov 12, 2024

There is no issue with a signer remaining delegated but inactive.

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 🤖_primary AI based primary recommendation 🤖_05_group AI based duplicate group recommendation unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

4 participants