Lybra Finance - 0xkazim's results

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/miner/ProtocolRewardsPool.sol#L87-L98 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/miner/ProtocolRewardsPool.sol#L88 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/miner/esLBRBoost.sol#L48

Vulnerability details

Impact

an attacker can set a boost to himself for 100% and locking his esLBR for 365 days using his esLBR that's mean the attacker has boost of 100% for his future esLBR tokens that he will mint it by calling stake in ProtocolRewards.sol if we say the attacker will burn 1 million LBR then he get amount of ESLBR with boost of 100% which mean a lot of tokens. and the attacker can't unStake his tokens until the boost/locked time ended in the unstake function. this is how it should work but that's not true. more details in POC

Proof of Concept

1-let's say that attacker(Alice) created a contract A and this contract have the functionality to call stake and unstake and before that Alice will get some esLBR and set the boost for this contract to %100 and then she call stake to burn the contract LBR and get esLBR with boost of 100%/365 day lock

2-Alice now have esLBR with boost 100% and the contract should prevent Alice to deposit her token until 365 days end. but Alice can bypass this check in the Unstake function by managing the contract A ownership, she can sell or set another address as the contract owner and before doing that she will delegate all the ESLBR voting power to her address and then she change or sell the contract ownership. the check in Unstake function:

function unstake(uint256 amount) external {
        //@audit
        require(block.timestamp >= esLBRBoost.getUnlockTime(msg.sender), "Your lock-in period has not ended. You can't convert your esLBR now.");
        esLBR.burn(msg.sender, amount);
        withdraw(msg.sender);
        uint256 total = amount;
        if (time2fullRedemption[msg.sender] > block.timestamp) {
            total += unstakeRatio[msg.sender] * (time2fullRedemption[msg.sender] - block.timestamp);
        }
        unstakeRatio[msg.sender] = total / exitCycle;
        time2fullRedemption[msg.sender] = block.timestamp + exitCycle;
        emit UnstakeLBR(msg.sender, amount, block.timestamp);
    }

as you see the require check the time lock for the msg.sender and if Alice managed the contract ownership then then esLBRBoost.getUnlockTime(msg.sender) will return zero because the new owner did not stake any tokens. in this case Alice have all esLBR voting power and she can get back her LBR by calling unstake function in less than 365 days/one year

the attack flow:

creating a contract that have functionallity to call stake/unstake and managing the ownership --> lock esLBR and get boost of 100% before everything --> call stake to mint esLBR with boost of %100 ---> delegate the esLBR voting power to Alice address --->sell or tokenize the ownership of the contract A and set a new address as owner ---> call the unstake to and bypass the check to return the LBR tokens

Tools Used

manual review

Recommended Mitigation Steps

I recommend to add mechanism to prevent this attack happen, according to the previous finding i read the white/black listing for contracts may work in this case. https://code4rena.com/reports/2022-03-paladin#h-02-system-could-be-wrapped-and-made-useless-without-contract-whitelisting

the mechanism that curveDAO used can be helpful too: https://github.com/curvefi/curve-dao-contracts/blob/4e428823c8ae9c0f8a669d796006fade11edb141/contracts/VotingEscrow.vy#L185

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraRETHVault.sol#L46-L48 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraRETHVault.sol#L27-L39

Vulnerability details

Impact

the function getAssetPrice in LybraRETHVault calling the getExchangeRatio in rETH contract which is not exist and the getAssetPrice will return nothing or revert and cause the depositEtherToMint function to revert or return invalid asset price.

Proof of Concept

the function getAssetPrice will call the getExchangeRatio in rETH/reth pool contract which is not exist

function getAssetPrice() public override returns (uint256) {
        //@audit this function not exist ! should use  getExchangeRate
        return (_etherPrice() * IRETH(address(collateralAsset)).getExchangeRatio()) / 1e18;
    }

and this function is called in depositEtherToMint function to mint eusd to the caller

    function depositEtherToMint(uint256 mintAmount) external payable override {
        require(msg.value >= 1 ether, "DNL");
        uint256 preBalance = collateralAsset.balanceOf(address(this));
        rkPool.deposit{value: msg.value}();
        uint256 balance = collateralAsset.balanceOf(address(this));
        depositedAsset[msg.sender] += balance - preBalance;


        if (mintAmount > 0) {
            _mintPeUSD(msg.sender, msg.sender, mintAmount, getAssetPrice());
        }


        emit DepositEther(msg.sender, address(collateralAsset), msg.value,balance - preBalance, block.timestamp);
    }

check the rETH contract address and rETH pool address(both don't have the getExchangeRatio function)

https://etherscan.io/address/0xae78736Cd615f374D3085123A210448E74Fc6393#code https://etherscan.io/address/0xDD3f50F8A6CafbE9b31a427582963f465E745AF8#code

Tools Used

manual review / etherscan

Recommended Mitigation Steps

the getAssetPrice function should call the getExchangeRate function in the rETH contract.

  // Get the current ETH : rETH exchange rate
    // Returns the amount of ETH backing 1 rETH
    function getExchangeRate() override external view returns (uint256) {
        return getEthValue(1 ether);
    }

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraStETHVault.sol#L101-L105 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraStETHVault.sol#L21-L28

Vulnerability details

Impact

in the function getDutchAuctionDiscountPrice in LybraStETHVault.sol it used to Reduces the discount for the issuance of additional tokens based on the rebase time. but this function missing a check for if the lidoRebaseTime is > than the block.timestamp because if we call this function and the block.timestamp is bigger than the lidoRebaseTime then this function will increase only.

Proof of Concept

if lidoRebaseTime is not greater than block.timestamp, it will forever increase and not decrease.

function getDutchAuctionDiscountPrice() public view returns (uint256) {
        //@audit
        //I think it should be lidoRebaseTime = block.timestamp + _time. Because you need the rebasetime to be greater than the current time
        uint256 time = (block.timestamp - lidoRebaseTime) % 1 days;
        if (time < 30 minutes) return 10000;
        return 10000 - (time / 30 minutes - 1) * 100;
    }

as you see in the function above there should be a check for the lidoRebaseTime > block.timestamp to avoid increasing instead of decreasing. even the lido rebase time is set by the owner so there is no guareente for if the the lidoRebaseTime are bigger than the block.timestamp.

    /**
     * @notice Sets the rebase time for Lido based on the actual situation.
     * This function can only be called by an address with the ADMIN role.
     */
    function setLidoRebaseTime(uint256 _time) external {
        //event miss here
        require(configurator.hasRole(keccak256("ADMIN"), msg.sender), "not authorized");
        lidoRebaseTime = _time;
    }

and even making lidoRebaseTime greater than block.timestamp for the time local variable will make the discount greater than if lidoRebaseTime wasn't greater than block.timestamp.

Tools Used

manual review

Recommended Mitigation Steps

i recommend to add check for the lidoRebaseTime > block.timestamp in getDutchAuctionDiscountPrice function.

or adding something like this will work: lidoRebaseTime = block.timestamp + _time

Assessed type

Other