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

Missing Whitelist Check in setApprovedBeneficiary #10

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

Missing Whitelist Check in setApprovedBeneficiary #10

c4-bot-6 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 unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-bot-6
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

  • Unauthorized Access: This vulnerability allows any user to modify the approved beneficiaries list, potentially granting unauthorized access to minting or redemption functions tied to these beneficiaries.
  • Loss of Trust: This breach in access control may erode trust in the contract and lead to unexpected and unauthorized modifications, weakening the overall security posture of the system.

Proof of Concept

Root Cause:
The setApprovedBeneficiary function lacks a check to ensure that the caller (msg.sender) is part of the _whitelistedBenefactors set. This missing verification allows any address to call setApprovedBeneficiary, thereby adding or removing a beneficiary without being a whitelisted benefactor. This oversight undermines the intended restriction on who can manage beneficiaries, posing a significant security risk.

  1. Issue in the Code:
    The setApprovedBeneficiary function currently does not have a require statement to check if msg.sender is in the _whitelistedBenefactors set.

    function setApprovedBeneficiary(address beneficiary, bool status) public {
        if (status) {
            if (!_approvedBeneficiariesPerBenefactor[msg.sender].add(beneficiary)) {
                revert InvalidBeneficiaryAddress();
            } else {
                emit BeneficiaryAdded(msg.sender, beneficiary);
            }
        } else {
            if (!_approvedBeneficiariesPerBenefactor[msg.sender].remove(beneficiary)) {
                revert InvalidBeneficiaryAddress();
            } else {
                emit BeneficiaryRemoved(msg.sender, beneficiary);
            }
        }
    }
  2. Proof of Unauthorized Call:
    Any unprivileged user can successfully execute the setApprovedBeneficiary function, altering the beneficiary list without restriction.

  3. Impact Illustration:

    • Example Attack Scenario: An attacker calls setApprovedBeneficiary from an unauthorized address to add a fraudulent beneficiary. This newly added beneficiary could be used for unauthorized minting or fund transfers, potentially causing financial loss or system manipulation.

Recommended Mitigation Steps

Add a Whitelist Check:
To mitigate this issue, add a require statement at the beginning of setApprovedBeneficiary to verify that the caller is in _whitelistedBenefactors:

function setApprovedBeneficiary(address beneficiary, bool status) public {
    require(_whitelistedBenefactors.contains(msg.sender), "Caller is not a whitelisted benefactor");

    if (status) {
        if (!_approvedBeneficiariesPerBenefactor[msg.sender].add(beneficiary)) {
            revert InvalidBeneficiaryAddress();
        } else {
            emit BeneficiaryAdded(msg.sender, beneficiary);
        }
    } else {
        if (!_approvedBeneficiariesPerBenefactor[msg.sender].remove(beneficiary)) {
            revert InvalidBeneficiaryAddress();
        } else {
            emit BeneficiaryRemoved(msg.sender, beneficiary);
        }
    }
}
@c4-bot-6 c4-bot-6 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-4 added a commit that referenced this issue Nov 11, 2024
@c4-bot-11 c4-bot-11 added the 🤖_primary AI based primary recommendation label Nov 11, 2024
@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Nov 12, 2024
@c4-judge
Copy link

0xEVom marked the issue as unsatisfactory:
Invalid

@0xEVom
Copy link

0xEVom commented Nov 12, 2024

The ability of benefactors to add beneficiaries and the benefactor whitelisting functionality are unrelated.

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 unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

4 participants