Blacklisted user can burn tokens in one case

[c4-bot-4]

Lines of code

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

Vulnerability details

Finding description and impact

When the USTB token operates in WHITELIST_ENABLED mode, users who hold both WHITELISTED and BLACKLISTED roles can still burn tokens, which violates expected behavior.

Proof of Concept

       } else if (transferState == TransferState.WHITELIST_ENABLED) {
            if (hasRole(MINTER_CONTRACT, msg.sender) && !hasRole(BLACKLISTED_ROLE, from) && to == address(0)) {
                // redeeming
            } else if (hasRole(MINTER_CONTRACT, msg.sender) && from == address(0) && !hasRole(BLACKLISTED_ROLE, to)) {
                // minting
            } else if (hasRole(DEFAULT_ADMIN_ROLE, msg.sender) && hasRole(BLACKLISTED_ROLE, from) && to == address(0)) {
                // redistributing - burn
            } else if (
                hasRole(DEFAULT_ADMIN_ROLE, msg.sender) && from == address(0) && !hasRole(BLACKLISTED_ROLE, to)
            ) {
                // redistributing - mint
>>          } else if (hasRole(WHITELISTED_ROLE, msg.sender) && hasRole(WHITELISTED_ROLE, from) && to == address(0)) {
                // whitelisted user can burn
            } else if (
                hasRole(WHITELISTED_ROLE, msg.sender) &&
                hasRole(WHITELISTED_ROLE, from) &&
                hasRole(WHITELISTED_ROLE, to) &&
                !hasRole(BLACKLISTED_ROLE, msg.sender) &&
                !hasRole(BLACKLISTED_ROLE, from) &&
                !hasRole(BLACKLISTED_ROLE, to)
            ) {
                // n.b. an address can be whitelisted and blacklisted at the same time
                // normal case

As seen above, the check allows a whitelisted user to burn tokens even if they are also blacklisted. Since the normal case branch acknowledges that an account can hold both WHITELISTED and BLACKLISTED roles, such users still have burning privileges, conflicting with one of the stated USTB invariants:

Blacklisted users cannot send/receive/mint/burn UStb tokens in any case.

Recommended mitigation steps

-           } else if (hasRole(WHITELISTED_ROLE, msg.sender) && hasRole(WHITELISTED_ROLE, from) && to == address(0)) {
+           } else if (
+               hasRole(WHITELISTED_ROLE, msg.sender) && 
+               hasRole(WHITELISTED_ROLE, from) && 
+               to == address(0) && 
+               !hasRole(BLACKLISTED_ROLE, msg.sender) && 
+               !hasRole(BLACKLISTED_ROLE, from)
+           ) {

[c4-judge]

0xEVom marked the issue as satisfactory

[c4-judge]

0xEVom marked the issue as primary issue

[iethena]

This can happen in 2 different variants:

1. A user is initially blacklisted but is later added to the whitelist without being removed from the blacklist
2. A user is initially whitelisted but is later added to the blacklist without being removed from the whitelist

In both cases the user can burn their tokens when whitelist mode is enabled for the transfer state. The likelihood of falling into this state is high because the Blacklist Manager and Whitelist Manager are intended to be different entities and as such we cannot guarantee that they will coordinate to maintain a clean separation of the blacklist/whitelist roles. If a user with a whitelist and blacklist role simultaneously, chose to burn their tokens in whitelist transfer state mode, it would have a positive impact on the protocol overall as there would be excess collateral in the protocol. We therefore believe that this issue should be marked as low as there is no incentive for a user to outright burn their tokens, in fact if a user does so, all other users in the protocol will benefit, without causing a negative impact.

[0xEVom]

This may have a positive impact on the protocol overall and even be intentional, but it does break one of the provided invariants. The consequences of bypassing a whitelist go beyond the immediate state changes (cf. sanctions), which must also be factored into the severity.

[c4-judge]

0xEVom marked the issue as selected for report

[KupiaSecAdmin]

@0xEVom! Thanks for your judging. I think this issue should be seen as QA based on the following C4 rule.

Centralization Risk

• Direct misuse of privileges shall be submitted in the QA report.

Both the BLACKLIST_MANAGER and WHITELIST_MANAGER are trusted roles; therefore, blacklisting a whitelisted user and whitelisting a blacklisted user are direct misuse of privileges.

In README.md:

All trusted roles in the protocol

| Role                  | Description                                                                        |
|-----------------------|------------------------------------------------------------------------------------|
| DEFAULT_ADMIN         | can assign roles to themselves or others and has the ability to perform any action |
| MINTER                | has the ability to call the mint function in UStb minting contract                 |
| REDEEMER              | has the ability to call the redeem function in UStb minting contract               |
| GATEKEEPER            | has the ability to disable minting and redeeming                                   |
| BLACKLIST_MANAGER  @> | has the ability to blacklist addresses                                             |
| WHITELIST_MANAGER  @> | has the ability to whitelist addresses                                             |
| COLLATERAL_MANAGER    | has the ability to withdraw collateral to custodians                               |

I submitted this issue for QA as a consequence.

[iethena]

Just to note that this was fixed in ethena-labs/ethena-usdtb-contest/pull/2 based off of a different qa finding which ensures that whitelist/blacklist roles are mutually exclusive.

[0xEVom]

There is no misuse of privileges here. As per the sponsor's comment above, both BLACKLIST_MANAGER and WHITELIST_MANAGER would be acting as intended.

It is the contract's inability to enforce the blacklisting functionality as designed (and as specified for this contest) that is an issue here.

[c4-judge]

0xEVom marked issue #15 as primary and marked this issue as a duplicate of 15