code-423n4 / 2024-02-spectra-findings

4 stars 2 forks source link

maxRedeem Function Fails to Return 0 When Protocol Is Paused #51

Closed c4-bot-8 closed 8 months ago

c4-bot-8 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L483-L485

Vulnerability details

Impact

The maxRedeem function is designed to return the maximum amount a user can redeem. However, it lacks a check to adjust its behavior when the protocol is paused, potentially misleading users or external contracts about the available actions during the pause state. This oversight may lead to failed transactions or unintended interactions, as users might attempt to redeem based on incorrect assumptions about their capabilities during a pause.

Proof of Concept

The issue lies within the maxRedeem function implementation in the PrincipalToken.sol contract. The function is intended to return the maximum amount that can be redeemed by a given owner. However, when the protocol is paused, the function does not account for this state and should, for security and clarity purposes, return 0 to indicate that no redemption actions are possible.

function maxRedeem(address owner) public view override returns (uint256) {
    return _maxBurnable(owner); //@audit must return 0 when protocol is paused
}

Tools Used

Manual

Recommended Mitigation Steps

To address this issue, it's recommended to modify the maxRedeem function to include a check for the protocol's paused state. If the protocol is paused, the function should immediately return 0, clearly indicating that no redemption actions are possible under the current state.

The modification could involve using a state variable or a function call to check the pause state, similar to how other functions might check for such conditions. Here is a conceptual implementation:

function maxRedeem(address owner) public view override returns (uint256) {
    if (protocolPaused()) { // This is a placeholder for the actual paused state check
        return 0;
    }
    return _maxBurnable(owner);
}

Assessed type

Context

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as duplicate of #33

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as sufficient quality report

c4-judge commented 8 months ago

JustDravee marked the issue as satisfactory

c4-judge commented 8 months ago

JustDravee marked the issue as partial-50