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

sNOTE Holders Are Not Incetivized To Vote On Proposals To Call extractTokensForCollateralShortfall #229

Open
code423n4 opened this issue Feb 3, 2022 · 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 sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

Handle

leastwood

Vulnerability details

Impact

As sNOTE have governance voting rights equivalent to the token amount in NOTE, users who stake their NOTE are also able to vote on governance proposals. In the event a majority of NOTE is staked in the sNOTE contract, it doesn't seem likely that stakers would be willing to vote on a proposal which liquidates a portion of their staked position.

Hence, the protocol may be put into a state where stakers are unwilling to vote on a proposal to call extractTokensForCollateralShortfall, leaving Notional insolvent as stakers continue to dump their holdings.

Proof of Concept

https://github.com/code-423n4/2022-01-notional/blob/main/contracts/sNOTE.sol#L99-L129

function extractTokensForCollateralShortfall(uint256 requestedWithdraw) external nonReentrant onlyOwner {
    uint256 bptBalance = BALANCER_POOL_TOKEN.balanceOf(address(this));
    uint256 maxBPTWithdraw = (bptBalance * MAX_SHORTFALL_WITHDRAW) / 100;
    // Do not allow a withdraw of more than the MAX_SHORTFALL_WITHDRAW percentage. Specifically don't
    // revert here since there may be a delay between when governance issues the token amount and when
    // the withdraw actually occurs.
    uint256 bptExitAmount = requestedWithdraw > maxBPTWithdraw ? maxBPTWithdraw : requestedWithdraw;

    IAsset[] memory assets = new IAsset[](2);
    assets[0] = IAsset(address(WETH));
    assets[1] = IAsset(address(NOTE));
    uint256[] memory minAmountsOut = new uint256[](2);
    minAmountsOut[0] = 0;
    minAmountsOut[1] = 0;

    BALANCER_VAULT.exitPool(
        NOTE_ETH_POOL_ID,
        address(this),
        payable(owner), // Owner will receive the NOTE and WETH
        IVault.ExitPoolRequest(
            assets,
            minAmountsOut,
            abi.encode(
                IVault.ExitKind.EXACT_BPT_IN_FOR_TOKENS_OUT,
                bptExitAmount
            ),
            false // Don't use internal balances
        )
    );
}

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider redesigning this mechanism to better align stakers with the health of the protocol. It might be useful to allocate a percentage of generated fees to an insurance fund which will be used to cover any collateral shortfall events. This fund can be staked to generate additional yield.

@code423n4 code423n4 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 Feb 3, 2022
code423n4 added a commit that referenced this issue Feb 3, 2022
@jeffywu jeffywu added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Feb 6, 2022
@jeffywu
Copy link
Collaborator

jeffywu commented Feb 6, 2022

Acknowledged, however, there are technical difficulties with programmatic collateral shortfall detection at this moment. We will look to develop a method that allows for programmatic detection in the future (these issues have been discussed with the warden).

@pauliax
Copy link
Collaborator

pauliax commented Feb 16, 2022

A hypothetical but valid concern.

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 sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

3 participants