Lybra Finance - naman1778's results

[N-01] Using Vulnerable Version of OpenZeppelin

The package.json configuration file says that the project is using 4.9.1 of OpenZeppelin which has a vulnerability.

Although there is no security vulnerability covering the project, it is recommended to use the latest version 4.9.2.

There is 1 instance of this issue in 1 file:

File: package.json

13:"@openzeppelin/contracts": "^4.9.1",

https://github.com/code-423n4/2023-06-lybra/blob/main/package.json

[N-02] Use a modifier for access control

Consider using a modifier to implement access control instead of inlining the condition/requirement in the function’s body.

There are 9 instances of this issue in 7 files:

File: contracts/lybra/token/esLBR.sol 

31: require(configurator.tokenMiner(msg.sender), "not authorized");

39: require(configurator.tokenMiner(msg.sender), "not authorized");

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/esLBR.sol

File: contracts/lybra/pools/LybraRETHVault.sol

42: require(configurator.hasRole(keccak256("TIMELOCK"), msg.sender));

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/LybraRETHVault.sol

File: contracts/lybra/token/LBR.sol	

25: function mint(address user, uint256 amount) external returns (bool) {
26:     require(configurator.tokenMiner(msg.sender), "not authorized");
27:     require(totalSupply() + amount <= maxSupply, "exceeding the maximum supply quantity.");
28:     _mint(user, amount);
29:     return true;
30: }

32: function burn(address user, uint256 amount) external returns (bool) {
33:     require(configurator.tokenMiner(msg.sender), "not authorized");
34:     _burn(user, amount);
35:     return true;
36: }

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/LBR.sol

File: contracts/lybra/pools/LybraStETHVault.sol 💰 📤 🧮 ♻️	

25: function setLidoRebaseTime(uint256 _time) external {
26:     require(configurator.hasRole(keccak256("ADMIN"), msg.sender), "not authorized");
27:    lidoRebaseTime = _time;
28: }

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/LybraStETHVault.sol

File: contracts/lybra/governance/LybraGovernance.sol 🧮	  

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

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/governance/LybraGovernance.sol

File: contracts/lybra/token/PeUSDMainnetStableVision.sol 💰	

79: function convertToPeUSD(address user, uint256 eusdAmount) public {
80:     require(_msgSender() == user || _msgSender() == address(this), "MDM");
81:     require(EUSD.balanceOf(address(this)) + eusdAmount <= configurator.getEUSDMaxLocked(),"ESL");
82:     bool success = EUSD.transferFrom(user, address(this), eusdAmount);
83:     require(success, "TF");
84:     uint256 share = EUSD.getSharesByMintedEUSD(eusdAmount);
85:     userConvertInfo[user].depositedEUSDShares += share;
86:     userConvertInfo[user].mintedPeUSD += eusdAmount;
87:     _mint(user, eusdAmount);
88: }

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/PeUSDMainnetStableVision.sol

File: contracts/lybra/miner/ProtocolRewardsPool.sol 📤	

227: function notifyRewardAmount(uint amount, uint tokenType) external {
228:     require(msg.sender == address(configurator));
229:     if (totalStaked() == 0) return;
230:     require(amount > 0, "amount = 0");
231:     if(tokenType == 0) {
232:         uint256 share = IEUSD(configurator.getEUSDAddress()).getSharesByMintedEUSD(amount);
233:         rewardPerTokenStored = rewardPerTokenStored + (share * 1e18) / totalStaked();
234:     } else if(tokenType == 1) {
235:         ERC20 token = ERC20(configurator.stableToken());
236:         rewardPerTokenStored = rewardPerTokenStored + (amount * 1e36 / token.decimals()) / totalStaked();
237:     } else {
238:         rewardPerTokenStored = rewardPerTokenStored + (amount * 1e18) / totalStaked();
239:     }
240: }

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/ProtocolRewardsPool.sol

[N-03] Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)

While the compiler knows to optimize away the exponentiation, it’s still better coding practice to use idioms that do not require compiler optimization, if they exist

There are 3 instances of this issue in 3 files:

File: contracts/lybra/token/PeUSD.sol	

14: ld2sdRatio = 10 ** (decimals - _sharedDecimals);

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/PeUSD.sol

File: contracts/lybra/token/LBR.sol	

22: ld2sdRatio = 10 ** (decimals - _sharedDecimals);

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/LBR.sol

File: contracts/lybra/token/PeUSDMainnetStableVision.sol

60: ld2sdRatio = 10 ** (decimals - _sharedDecimals);

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/PeUSDMainnetStableVision.sol

[N-04] According to the syntax rules, use => mapping ( instead of => mapping( using spaces as keyword

There is 1 instance of this issue in 1 file:

File: contracts/lybra/token/EUSD.sol

56: mapping(address => mapping(address => uint256)) private allowances;

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/EUSD.sol

[N-05] Inconsistent Solidity Versions

Instead of the below mentioned file, all other files use 0.8.17 version of solidity.

There is 1 instance of this issue in 1 file:

File: contracts/lybra/miner/stakerewardV2pool.sol

2: pragma solidity ^0.8;

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/stakerewardV2pool.sol

[N-06] Empty/Unused Function Parameters

Empty or unused function parameters should be commented out as a better and declarative way to silence runtime warning messages. As an example, the following function may have these parameters refactored to:

There are 9 instances of this issue in 4 files:

File: contracts/lybra/token/PeUSD.sol	

31: function _debitFrom(address _from, uint16, bytes32, uint _amount) internal virtual override returns (uint) {

38: function _creditTo(uint16, address _toAddress, uint _amount) internal virtual override returns (uint) {

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/PeUSD.sol

File: contracts/lybra/token/LBR.sol	

49: function _debitFrom(address _from, uint16, bytes32, uint _amount) internal virtual override returns (uint) {

56: function _creditTo(uint16, address _toAddress, uint _amount) internal virtual override returns (uint) {

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/LBR.sol

File: contracts/lybra/governance/LybraGovernance.sol 🧮	

76: function _countVote(uint256 proposalId, address account, uint8 support, uint256 weight, bytes memory) internal override {

98: function _getVotes(address account, uint256 timepoint, bytes memory) internal view override returns (uint256){

106: function _execute(uint256 /* proposalId */, address[] memory targets, uint256[] memory values, bytes[] memory calldatas, bytes32 descriptionHash) internal virtual override {

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/governance/LybraGovernance.sol

File: contracts/lybra/token/PeUSDMainnetStableVision.sol 💰	

184: function _debitFrom(address _from, uint16, bytes32, uint _amount) internal virtual override returns (uint) {

191: function _creditTo(uint16, address _toAddress, uint _amount) internal virtual override returns (uint) {

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/PeUSDMainnetStableVision.sol

[G-01] Use assembly to check for address(0)

Checking zero address can be improved by replacing the require statement with Assembly.Solidity has a lot of guardrails that can be removed (with care) for optimization purposes, especially for simple functionality like checking if an address is zero.

There are 18 instances of this issue in 7 files:

File: contracts/lybra/miner/stakerewardV2pool.sol

60: if (_account != address(0)) {

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/stakerewardV2pool.sol

File: contracts/lybra/token/PeUSDMainnetStableVision.sol

64: require(to != address(0), "TZA");

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/PeUSDMainnetStableVision.sol

File: contracts/lybra/token/EUSD.sol

367: require(_owner != address(0), "APPROVE_FROM_ZERO_ADDRESS");

368: require(_spender != address(0), "APPROVE_TO_ZERO_ADDRESS");

392: require(_sender != address(0), "TRANSFER_FROM_THE_ZERO_ADDRESS");

393: require(_recipient != address(0), "TRANSFER_TO_THE_ZERO_ADDRESS");

412: require(_recipient != address(0), "MINT_TO_THE_ZERO_ADDRESS");

441: require(_account != address(0), "BURN_FROM_THE_ZERO_ADDRESS");

460: require(_account != address(0), "BURN_FROM_THE_ZERO_ADDRESS");

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/EUSD.sol

File: contracts/lybra/configuration/LybraConfigurator.sol

99: if (address(EUSD) == address(0)) EUSD = IEUSD(_eusd);

100: if (address(peUSD) == address(0)) peUSD = IEUSD(_peusd);

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/configuration/LybraConfigurator.sol

File: contracts/lybra/miner/EUSDMiningIncentives.sol 	

76: if (_account != address(0)) {

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/EUSDMiningIncentives.sol

File: contracts/lybra/pools/base/LybraEUSDVaultBase.sol

99: require(onBehalfOf != address(0), "TZA");

127: require(onBehalfOf != address(0), "MINT_TO_THE_ZERO_ADDRESS");

141: require(onBehalfOf != address(0), "BURN_TO_THE_ZERO_ADDRESS");

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/base/LybraEUSDVaultBase.sol

File: contracts/lybra/pools/base/LybraPeUSDVaultBase.sol

83: require(onBehalfOf != address(0), "TZA");

97: require(onBehalfOf != address(0), "TZA");

111: require(onBehalfOf != address(0), "TZA"); 

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol

Test Code

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;
    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }
    function testGas() public {
        c0.solidity_notZero(0xFD2dabe9DFcc4d88a12A9D0D40D834E81217Cccf);
        c1.assembly_notZero(0xFD2dabe9DFcc4d88a12A9D0D40D834E81217Cccf);
    }
}


contract Contract0 {

    error ZeroAddress();

    function solidity_notZero(address toCheck) public pure returns(bool success) {
        if(toCheck == address(0)) revert ZeroAddress();

        return true;
    }
}

contract Contract1{

error ZeroAddress();
function assembly_notZero(address toCheck) public pure returns(bool success) {
    assembly {
        if iszero(toCheck) {
            let ptr := mload(0x40)
            mstore(ptr, 0xd92e233d00000000000000000000000000000000000000000000000000000000) // selector for `ZeroAddress()`
            revert(ptr, 0x4)
        }
    }
    return true;
}
}

Gas Report

[G-02] Use assembly to write address storage values

When writing value for variables whose type is address, make use of assembly code instead of solidity code.

There are 6 instances of this issue in 2 files:

File: contracts/lybra/configuration/LybraConfigurator.sol

262: stableToken = _token;

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/configuration/LybraConfigurator.sol

File: contracts/lybra/miner/EUSDMiningIncentives.sol 	

85: LBR = _lbr;

86: esLBR = _eslbr;

97: pools = _pools;

125: ethlbrStakePool = _pool;

126: ethlbrLpToken = _lp;

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/EUSDMiningIncentives.sol

Test Code

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;

    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }

    function testGas() public {
        c0.setOwnerAssembly(0xFD2dabe9DFcc4d88a12A9D0D40D834E81217Cccf);
        c1.setOwner(0xFD2dabe9DFcc4d88a12A9D0D40D834E81217Cccf);
    }
}

contract Contract0 {

    address owner;
    function setOwnerAssembly(address _owner) public {
        assembly{
            sstore(owner.slot,_owner)
        }
    }

}

contract Contract1 {
    address owner;
    function setOwner(address _owner) public {
        owner = _owner;
    }

}

Gas Report

[G-03] Use nested if and, avoid multiple check combinations

Using nesting is cheaper than using && multiple check combinations. There are more advantages, such as easier to read code and better coverage reports.

There are 4 instances of this issue in 4 files:

File: contracts/lybra/token/PeUSD.sol	

46: if (_from != address(this) && _from != spender)

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/PeUSD.sol

File: contracts/lybra/token/LBR.sol	

64: if (_from != address(this) && _from != spender)

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/LBR.sol

File: contracts/lybra/token/PeUSDMainnetStableVision.sol

199: if (_from != address(this) && _from != spender)

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/PeUSDMainnetStableVision.sol

File: contracts/lybra/pools/base/LybraEUSDVaultBase.sol

204: if (msg.sender != provider && onBehalfOfCollateralRatio >= 1e20 + configurator.vaultKeeperRatio(address(this)) * 1e18) {

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/base/LybraEUSDVaultBase.sol

Test Code

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;

    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }

    function testGas() public {
        c0.checkAge(19);
        c1.checkAgeOptimized(19);
    }
}

contract Contract0 {

    function checkAge(uint8 _age) public returns(string memory){
        if(_age>18 && _age<22){
            return "Eligible";
        }
    }

}

contract Contract1 {

    function checkAgeOptimized(uint8 _age) public returns(string memory){
        if(_age>18){
            if(_age<22){
                return "Eligible";
            }
        }
    }
}

Gas Report

[G-04] Using XOR (^) and AND (&) bitwise equivalents /// incomplete

On Remix, given only uint256 types, the following are logical equivalents, but don’t cost the same amount of gas:

(a != b || c != d || e != f) costs 571 ((a ^ b) | (c ^ d) | (e ^ f)) != 0 costs 498 (saving 73 gas) Consider rewriting as following to save gas:

To have a == b means that every 0 and 1 match on both variables. Meaning that a XOR (operator ^) would evaluate to 0 ((a ^ b) == 0), as it excludes by definition any equalities. Now, if a != b, this means that there’s at least somewhere a 1 and a 0 not matching between a and b, making (a ^ b) != 0.

Both formulas are logically equivalent and using the XOR bitwise operator costs actually the same amount of gas:

However, it is much cheaper to use the bitwise OR operator (|) than comparing the truthy or falsy values

There is 1 instance of this issue in 1 file:

File: contracts/lybra/token/PeUSDMainnetStableVision.sol

80: require(_msgSender() == user || _msgSender() == address(this), "MDM");

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/PeUSDMainnetStableVision.sol

Test Code

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;
    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }
    function testGas() public {
        c0.not_optimized(1,2);
        c1.optimized(1,2);
    }
}

contract Contract0 {
    function not_optimized(uint8 a,uint8 b) public returns(bool){
        return ((a==1) || (b==1));
    }
}

contract Contract1 {
    function optimized(uint8 a,uint8 b) public returns(bool){
        return ((a ^ 1) & (b ^ 1)) == 0;
    }
}

Gas Report

[G-05] Instead of counting down in for statements, count up

Counting down is more gas efficient than counting up because neither we are making zero variable to non-zero variable and also we will get gas refund in the last transaction when making non-zero to zero variable.

There are 3 instances of this issue in 3 files:

File: contracts/lybra/configuration/LybraConfigurator.sol

236: for (uint256 i = 0; i < _contracts.length; i++) {

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/configuration/LybraConfigurator.sol

File: contracts/lybra/miner/EUSDMiningIncentives.sol 	

94: for (uint i = 0; i < _pools.length; i++) {

138: for (uint i = 0; i < pools.length; i++) {

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/EUSDMiningIncentives.sol

Test Code

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;
    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }
    function testGas() public {
        c0.AddNum();
        c1.AddNum();
    }
}


contract Contract0 {
    uint256 num = 3;
    function AddNum() public {
        uint256 _num = num;
        for(uint i=0;i<=9;i++){
            _num = _num +1;
        }
        num = _num;
    }
}


contract Contract1 {
    uint256 num = 3;
    function AddNum() public {
        uint256 _num = num;
        for(uint i=9;i>=0;i--){
            _num = _num +1;
        }
        num = _num;
    }
}

Gas Report

[G-06] Using calldata instead of memory for read-only arguments in external functions saves gas

When a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution. Note that even if an interface defines a function as having memory arguments, it’s still valid for implementation contracs to use calldata arguments instead.

If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it’s still more gass-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one

Note that I’ve also flagged instances where the function is public but can be marked as external since it’s not called by the contract, and cases where a constructor is involved

There are 2 instances of this issue in 2 files:

File: contracts/lybra/miner/esLBRBoost.sol	

33: function addLockSetting(esLBRLockSetting memory setting) external onlyOwner {

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/esLBRBoost.sol

File: contracts/lybra/miner/EUSDMiningIncentives.sol 	

93: function setPools(address[] memory _pools) external onlyOwner {

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/EUSDMiningIncentives.sol

Test Code

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;
    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }
    function testGas() public {
        c0.not_optimized("Naman");
        c1.optimized("Naman");
    }
}

contract Contract0 {

     function not_optimized(string memory a) public returns(string memory){
         return a;
     }
}

contract Contract1 {

     function optimized(string calldata a) public returns(string calldata){
         return a;
     }
}

Gas Report

[G-07] Stack variable used as a cheaper cache for a state variable is only used once

If the variable is only accessed once, it’s cheaper to use the state variable directly that one time, and save the 3 gas the extra stack assignment would spend.

There are 4 instances of this issue in 4 files:

File: contracts/lybra/miner/stakerewardV2pool.sol

136: uint256 remainingRewards = (finishAt - block.timestamp) * rewardRatio;

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/stakerewardV2pool.sol

File: contracts/lybra/miner/EUSDMiningIncentives.sol 	

233: uint256 remainingRewards = (finishAt - block.timestamp) * rewardRatio;

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/EUSDMiningIncentives.sol

File: contracts/lybra/pools/base/LybraEUSDVaultBase.sol

236: uint256 providerCollateralRatio = (depositedAsset[provider] * assetPrice * 100) / borrowed[provider];

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/base/LybraEUSDVaultBase.sol

File: contracts/lybra/pools/base/LybraVaultBase.sol

161: uint256 providerCollateralRatio = (depositedAsset[provider] * assetPrice * 100) / borrowed[provider];

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol

Test Code

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;
    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }
    function testGas() public {
        c0.not_optimized();
        c1.optimized();
    }
}

contract Contract0 {
    uint256 num = 5;
    uint256 sum;
    function not_optimized() public returns(bool){
        uint256 num1 = num;
        sum = num1 + 2;
    }
}

contract Contract1 {
    uint256 num = 5;
    uint256 sum;
    function optimized() public returns(bool){
        sum = num + 2;
    }
}

Gas Report

[G-08] Multiplication by two should use bit shifting

<x> * 2 is the same as <x> << 1. While the compiler uses the SHR opcode to accomplish both, the version that uses division incurs an overhead of 20 gas due to JUMPs to and from a compiler utility function that introduces checks which can be avoided by using unchecked {} around the division by two.

There are 2 instances of this issue in 2 files:

File: contracts/lybra/pools/base/LybraEUSDVaultBase.sol

159: require(assetAmount * 2 <= depositedAsset[onBehalfOf], "a max of 50% collateral can be liquidated");

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/base/LybraEUSDVaultBase.sol

File: contracts/lybra/pools/base/LybraPeUSDVaultBase.sol

130: require(assetAmount * 2 <= depositedAsset[onBehalfOf], "a max of 50% collateral can be liquidated");

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol

Test Code

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;
    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }
    function testGas() public {
        c0.not_optimized();
        c1.optimized();
    }
}

contract Contract0 {
    uint256 num = 5;
    uint256 mul;
    function not_optimized() public returns(bool){
        mul = num * 2;
    }
}

contract Contract1 {
    uint256 num = 5;
    uint256 mul;
    function optimized() public returns(bool){
        mul = num << 2;
    }
}

Gas Report