Lybra Finance - Kaysoft's results

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/EUSD.sol#L411

Vulnerability details

Impact

The EUSD.sol contract is vulnerable to "ERC4626 inflation attack". The EUSD.sol contract is susceptible to an underlying balance manipulation attack When a user performs mint operation, the amount of shares they receive is calculated by taking the number of assets provided, multiplying that by the number of existing shares in circulation and then dividing the result by all assets owned by the smart contract vault.

Proof of Concept

The attack is carried out by manipulating the totalShares by minting a small amount of shares and fruntrunning the next user that mints.

function getMintedEUSDByShares(uint256 _sharesAmount) public view returns (uint256) {
        if (_totalShares == 0) {
            return 0;
        } else {
            return _sharesAmount.mul(_totalSupply).div(_totalShares);
        }
    }

Attack Scenario:

1. The hacker mints for herself one share: deposit(1). Thus, totalAsset()==1, totalSupply()==1.
2. The hacker front-runs the deposit of the victim who wants to deposit 20,000 USDT (20,000.000000).
3. The hacker inflates the denominator right in front of the victim: asset.transfer(20_000e6). Now totalAsset()==20_000e6 + 1, totalSupply()==1.
4. Next, the victim's tx takes place. The victim gets 1 * 20_000e6 / (20_000e6 + 1) == 0 shares. The victim gets zero shares.
5. The hacker burns their share and gets all the money.

see further examples of this attack:

• https://mixbytes.io/blog/overview-of-the-inflation-attack
• https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/audits/2022-10-ERC4626.pdf

Tools Used

Manual Review

Recommended Mitigation Steps

Use Openzeppelin's implementation of ERC4626: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/8177c4620e049b2749c2069651d7d5b4691e23d2/contracts/token/ERC20/extensions/ERC4626.sol

Assessed type

ERC4626

[L-1] Refactor _countVote function to remove the unnamed parameter.

The last parameter of _countVote function in LibraGovernance.sol is unnamed. Refactor code to remove unnamed parameter.

File: https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/governance/LybraGovernance.sol#L76

function _countVote(uint256 proposalId, address account, uint8 support, uint256 weight, bytes memory) internal override {
      
        require(state(proposalId) == ProposalState.Active, "GovernorBravo::castVoteInternal: voting is closed");
        require(support <= 2, "GovernorBravo::castVoteInternal: invalid vote type");
        ProposalExtraData storage proposalExtraData = proposalData[proposalId];
        Receipt storage receipt = proposalExtraData.receipts[account];
        require(receipt.hasVoted == false, "GovernorBravo::castVoteInternal: voter already voted");
        
        proposalExtraData.supportVotes[support] += weight;
       

        receipt.hasVoted = true;
        receipt.support = support;
        receipt.votes = weight;
        proposalExtraData.totalVotes += weight;
        
    }

[L-2] Refactor code to remove commented variable

The proposalId parameter of the _execute function in LibraGovernance.sol is commented out. Refactor code to remove commented parameter.

File: https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/governance/LybraGovernance.sol#L106

function _execute(uint256 /* proposalId */,  address[] memory targets, uint256[] memory values, bytes[] memory calldatas, bytes32 descriptionHash) internal virtual override {
        require(GovernanceTimelock.checkOnlyRole(keccak256("TIMELOCK"), msg.sender), "not authorized");
        super._execute(1, targets, values, calldatas, descriptionHash);
        // _timelock.executeBatch{value: msg.value}(targets, values, calldatas, 0, descriptionHash);//@audit commented code.
    }

[L-3] Remove commented code

Remove commented code.

File: https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/governance/LybraGovernance.sol#L109

 function _execute(uint256 /* proposalId */, address[] memory targets, uint256[] memory values, bytes[] memory calldatas, bytes32 descriptionHash) internal virtual override {
        require(GovernanceTimelock.checkOnlyRole(keccak256("TIMELOCK"), msg.sender), "not authorized");
        super._execute(1, targets, values, calldatas, descriptionHash);
        // _timelock.executeBatch{value: msg.value}(targets, values, calldatas, 0, descriptionHash);
    }

[L-4] Do not hardcode address since contracts will be deployed to multiple chains.

The stakedLBRLpValue function in the EUSDMinningIncentives.sol uses a hardcoded address. Do not hardcode addresses since the contract will be deployed to multiple network.

File: https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/miner/EUSDMiningIncentives.sol#L153

function stakedLBRLpValue(address user) public view returns (uint256) {
        uint256 totalLp = IEUSD(ethlbrLpToken).totalSupply();
        (, int etherPrice, , , ) = etherPriceFeed.latestRoundData();
        (, int lbrPrice, , , ) = lbrPriceFeed.latestRoundData();
        uint256 etherInLp = (IEUSD(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2).balanceOf(ethlbrLpToken) * uint(etherPrice)) / 1e8;
        uint256 lbrInLp = (IEUSD(LBR).balanceOf(ethlbrLpToken) * uint(lbrPrice)) / 1e8;
        uint256 userStaked = IEUSD(ethlbrStakePool).balanceOf(user);
        return (userStaked * (lbrInLp + etherInLp)) / totalLp;
    }

[L-5] Variable Shadowing in the constructors of the listed files below.

The decimals local variable shadows the ERC20.decimals state variable of the parent contract.

Use another local variable name in order to avoid variable shadowing. Files:

• https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/LBR.sol#L20
• https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/PeUSD.sol#L12
• https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/PeUSDMainnetStableVision.sol#L58

constructor(address _config, uint8 _sharedDecimals, address _lzEndpoint)
        ERC20("LBR", "LBR")
        BaseOFTV2(_sharedDecimals, _lzEndpoint)
    {
        configurator = Iconfigurator(_config);
        uint8 decimals = decimals(); //@audit shadowed variables - check for further attack.
        require(_sharedDecimals <= decimals, "OFT: sharedDecimals must be <= decimals");
        ld2sdRatio = 10 ** (decimals - _sharedDecimals);
    }

[NC-1] Use a more recent version of solidity

Use a solidity version of at least 0.8.19 as latest stable versions will bug fixes and security updates. Files: most files.