code-423n4 / 2022-03-sublime-findings

0 stars 0 forks source link

QA Report #61

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

[L] LenderPool.sol#lend() Wrong event emitted

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/LenderPool.sol#L281-L304

    function lend(uint256 _id, uint256 _amount) external nonReentrant {
        require(VERIFICATION.isUser(msg.sender, pooledCLConstants[_id].lenderVerifier), 'L1');
        require(block.timestamp < pooledCLConstants[_id].startTime, 'L2');

        uint256 _totalLent = totalSupply[_id];
        uint256 _maxLent = pooledCLConstants[_id].borrowLimit;
        require(_maxLent > _totalLent, 'L3');

        uint256 _amountToLend = _amount;
        if (_totalLent.add(_amount) > _maxLent) {
            _amountToLend = _maxLent.sub(_totalLent);
        }
        address _borrowAsset = pooledCLConstants[_id].borrowAsset;

        IERC20(_borrowAsset).safeTransferFrom(msg.sender, address(this), _amountToLend);
        _mint(msg.sender, _id, _amountToLend, '');

        emit Lend(_id, msg.sender, _amount);

        // If borrowLimit reached, accept CL
        if (_totalLent.add(_amountToLend) == _maxLent) {
            _accept(_id, _maxLent);
        }
    }

_amount should be changed to : _amountToLend.

[L] Wrong value for LimitsUpdated event parameter

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L233

    event LimitsUpdated(string indexed limitType, uint256 max, uint256 min);

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L404-L409

    function updateBorrowLimitLimits(uint256 _min, uint256 _max) external onlyOwner {
        require(_min < _max || _min.mul(_max) == 0, 'UBLL1');
        require(!(borrowLimitLimits.min == _min && borrowLimitLimits.max == _max), 'UBLL2');
        borrowLimitLimits = Limits(_min, _max);
        emit LimitsUpdated('borrowLimit', _min, _max);
    }

Recommendation

Change to:

        emit LimitsUpdated('borrowLimit', _max, _min);

[N] Outdated compiler version

It's a best practice to use the latest compiler version.

The specified minimum compiler version is quite old. Older compilers might be susceptible to some bugs. We recommend changing the solidity version pragma to the latest version to enforce the use of an up to date compiler.

List of known compiler bugs and their severity can be found here: https://etherscan.io/solcbuginfo

[N] PooledCreditLine.sol LenderPool.sol should use the Upgradeable variant of OpenZeppelin Contracts

As per the OpenZeppelin docs:

If your contract is going to be deployed with upgradeability, such as using the OpenZeppelin Upgrades Plugins, you will need to use the Upgradeable variant of OpenZeppelin Contracts.

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L7-L7

import '@openzeppelin/contracts/utils/ReentrancyGuard.sol';

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/LenderPool.sol#L7-L7

import '@openzeppelin/contracts/utils/ReentrancyGuard.sol';

Recommendation

Change to:

import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";

[L] Precision loss due to div before mul

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L944

_maxPossible = _totalCollateralToken.mul(_ratioOfPrices).div(_collateralRatio).mul(SCALING_FACTOR).div(10**_decimals);

Can be changed to:

_maxPossible = _totalCollateralToken.mul(_ratioOfPrices).mul(SCALING_FACTOR).div(_collateralRatio).div(10**_decimals);

[N] Fee on transfer tokens are not supported

ritik99 commented 2 years ago

All suggestions except second last ("[L] Precision loss due to div before mul") are valid. The operations have been defined in that sequence to prevent overflows.

The report is of high quality