sherlock-audit / 2023-01-sentiment-judging

2 stars 0 forks source link

DecorativePineapple - The `PLVGLPOracle:getPrice()` function is vulnerable to price manipulation as it depended on the `previewRedeem` function #13

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

DecorativePineapple

medium

The PLVGLPOracle:getPrice() function is vulnerable to price manipulation as it depended on the previewRedeem function

Summary

It has been identified that the PLVGLPOracle:getPrice() function is vulnerable to price manipulation as it uses the ERC4626 previewRedeem function to calculate the asset price.

Vulnerability Detail

The getPrice() function within the PLVGLPOracle contract is vulnerable to price manipulation because the price can be increased or decreased within a single transaction/block, in the same block that the getPrice() function is called.

The previewRedeem function is calculated as:

function previewRedeem(uint256 shares) public view virtual returns (uint256) {
        return convertToAssets(shares);
}

The convertToAssets function:

    function convertToAssets(uint256 shares) public view virtual returns (uint256) {
        uint256 supply = totalSupply; 

        return supply == 0 ? shares : shares.mulDivDown(totalAssets(), supply);
    }

As the convertToAssets is calculated by multiplying the shares with the totalAssets() and then dividing with the totalSupply while rounding down, it can be increased or decreased within a single block/transaction by calling the vault's deposit, mint, withdraw or redeem functions. This allows the attacker to artificially inflate or deflate the price within a single block/transaction.

Impact

The attacker could manipulate the price of the vault in order to make the value of an asset to be much higher or much lower than the true value of the asset. This leads to the manipulation of the getPrice() function.

Code Snippet

The getPrice() function:

/// @inheritdoc IOracle
    function getPrice(address) external view returns (uint) {
        (,, uint assets) = vault.previewRedeem(address(0), 1e18);
        return assets.mulWadDown(
            oracleFacade.getPrice(sGLP)
        );
    }

Tool used

Manual Review

Recommendation

It is recommended to avoid using the previewRedeem function to calculate the price of the asset. You could implement a TWAP, so the price cannot be inflated/deflated within a single transaction.