Lybra Finance - lanrebayode77's results

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/5d70170f2c68dbd3f7b8c0c8fd6b0b2218784ea6/contracts/lybra/configuration/LybraConfigurator.sol#L85-L93

Vulnerability details

Impact

In LybraConfigurator.sol contract, all restricted functions are callable by any address, making the entire protocol vulnerable to severe manipulation by a malicious user.

Proof of Concept

Modifiers are unchecked .

 modifier onlyRole(bytes32 role) {
        GovernanceTimelock.checkOnlyRole(role, msg.sender);
        _;
    }

    modifier checkRole(bytes32 role) {
        GovernanceTimelock.checkRole(role, msg.sender);
        _;
    }

Tools Used

Manual Finding.

Recommended Mitigation Steps

use require to validate.

 modifier onlyRole(bytes32 role) {
        require(GovernanceTimelock.checkOnlyRole(role, msg.sender), "Restricted");
        _;
    }

    modifier checkRole(bytes32 role) {
        require(GovernanceTimelock.checkRole(role, msg.sender), "Restricted");
        _;
    }

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/5d70170f2c68dbd3f7b8c0c8fd6b0b2218784ea6/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L192-L210

Vulnerability details

Impact

Users will payback less than the actual borrowed amount, couple with the fact that they will be able to withdrawal slightly above what ought to be acceptable by their safeCollateralratio because borrowed[_onBehalfOf] has reduce slightly than expected.

Proof of Concept

 function _repay(address _provider, address _onBehalfOf, uint256 _amount) internal virtual {
        try configurator.refreshMintReward(_onBehalfOf) {} catch {}
        _updateFee(_onBehalfOf);
        uint256 totalFee = feeStored[_onBehalfOf];
        uint256 amount = borrowed[_onBehalfOf] + totalFee >= _amount ? _amount : borrowed[_onBehalfOf] + totalFee;
        if(amount >= totalFee) {
            feeStored[_onBehalfOf] = 0;
            PeUSD.transferFrom(_provider, address(configurator), totalFee);
            PeUSD.burn(_provider, amount - totalFee);
        } else {
            feeStored[_onBehalfOf] = totalFee - amount;
            PeUSD.transferFrom(_provider, address(configurator), amount);
        }
        try configurator.distributeRewards() {} catch {}
        borrowed[_onBehalfOf] -= amount;
        poolTotalPeUSDCirculation -= amount;

        emit Burn(_provider, _onBehalfOf, amount, block.timestamp);
    }

The borrowed[_onBehalfOf] is being updated without considering the amount paid as fees.

Tools Used

Manual review.

Recommended Mitigation Steps

The amount that should be subtracted from the borrowed[_onBehalf] is the remaining balance after removing totalFees paid from the amount variable. borrowed[_onBehalfOf] -= (amount - totalFee);

Assessed type

Math