0xEkko - Lack of Access Control in `onNewIssuance` Function Allows Unauthorized Liquidity Manipulation and Forced LP Liquidation

[sherlock-admin2]

0xEkko

High

Lack of Access Control in onNewIssuance Function Allows Unauthorized Liquidity Manipulation and Forced LP Liquidation

Summary

The onNewIssuance function in VaultLibrary.sol is marked as external without proper access control. As a result, any malicious actor can call this function directly, leading to potential unauthorized actions such as liquidity manipulation or liquidating LP positions. These actions could severely destabilize the protocol by allowing unauthorized minting, liquidity provisioning, or even triggering liquidations that could harm both users and the protocol.

Vulnerability Detail

In the ModuleCore.sol contract, the function issueNewDs correctly uses the onlyConfig modifier to restrict access, ensuring only authorized parties can invoke the function. However, inside issueNewDs, the vulnerable VaultLibrary.onNewIssuance function is called, which is marked as external and lacks proper access control. This exposes the system to unauthorized actions, as any external party can directly call onNewIssuance, bypassing intended restrictions.

function issueNewDs(Id id, uint256 expiry, uint256 exchangeRates, uint256 repurchaseFeePrecentage)
    external
    override
    onlyConfig
    onlyInitialized(id)
{
    // Function logic...
    VaultLibrary.onNewIssuance(state, prevIdx, getRouterCore(), getAmmRouter()); // External call
}

The onNewIssuance() function, when called, interacts with other critical functions like _liquidateLp and __provideAmmLiquidityFromPool, which can be exploited for unauthorized liquidity manipulation or liquidation of LP positions.

Impact

A malicious actor could directly call the onNewIssuance() function, triggering _liquidateLp which allows a user to forcibly liquidate an LP position. This could lead to the premature liquidation of liquidity providers' positions, resulting in loss of assets for legitimate users and potentially causing a negative ripple effect in the liquidity pools.

Code Snippet

    function onNewIssuance(
        State storage self,
        uint256 prevDsId,
        IDsFlashSwapCore flashSwapRouter,
        IUniswapV2Router02 ammRouter
    ) external {
        // do nothing at first issuance
        if (prevDsId == 0) {
            return;
        }

        if (!self.vault.lpLiquidated.get(prevDsId)) {
            _liquidatedLp(self, prevDsId, ammRouter, flashSwapRouter);
        }

        __provideAmmLiquidityFromPool(self, flashSwapRouter, self.ds[self.globalAssetIdx].ct, ammRouter);
    }

Note: This poses a significant risk as it can trigger unauthorized actions such as _liquidatedLp, which liquidates liquidity providers’ positions, or _provideAmmLiquidityFromPool, which affects liquidity provisioning.

Tool used

Manual Review

Recommendation

Add appropriate access control mechanisms to restrict access to the onNewIssuance function in VaultLibrary.sol

[sherlock-admin4]

1 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

This is a Library function.