Sublime contest - 0xkatana's results

1. Low - Possible casting overflow

Impact

A casting overflow can happen when a uint256 is cast to a uint128. SafeMath does not provide protection for casting overflows, and the overflow may lead to unexpected edge cases.

Proof of Concept

A casting overflow could occur at this line of PooledCreditLine https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L723

An amount value this large may be reached for some ERC20 tokens with a very large total supply and a very small value per token.

Tools Used

Manual analysis

Recommended Mitigation Steps

Make borrowLimit uint256, not uint128 in the PooledCreditLineConstants struct. The only reason why borrowLimit and borrowRate in the PooledCreditLineConstants struct are uint128 is for gas savings, but it would be safer to use uint256 for these variables. The gas savings might be minor because the casting operation is required.

Another solution is to use Open Zeppelin SafeCast

2. Low - transfer return value is ignored

Impact

Some ERC20 tokens, such as USDT, don't revert when transfer/transferFrom fails. The transfer return value has to be checked (as there are some other tokens that returns false instead revert). safeTransfer should be used instead of transfer

Proof of Concept

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

Tools Used

Manual analysis

Recommended Mitigation Steps

Use safeTransfer instead of transfer. This line using SafeERC20 for IERC20; means the safeTransfer function is available

3. Low - Collateral can be added when expired

Impact

The PoolCreditLine contract allows collateral deposits when the PooledCreditLineStatus is expired. There should not be any need to add collateral when the status is expired, so this logic branch should be removed to reduce possible edge case scenarios.

Proof of Concept

The suspect line of code is https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L745

Tools Used

Manual analysis

Recommended Mitigation Steps

Remove the code || _status == PooledCreditLineStatus.EXPIRED from the if statement of line 745 of PoolCreditLine

1. Use simple comparison

Impact

The comparison operators >= and <= use more gas than >, <, or ==. Replacing the >= and ≤ operators with a comparison operator that has an opcode in the EVM saves gas

Proof of Concept

This code is in calculateBorrowableAmount from PooledCreditLine is the same as an if/else statement https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L950-L952

if (_maxPossible <= _currentDebt) return 0;

return Math.min(_borrowLimit - _principal, _maxPossible - _currentDebt);

A simple comparison can be used for gas savings by reversing the logic

if (_maxPossible > _currentDebt) return Math.min(_borrowLimit - _principal, _maxPossible - _currentDebt);

return 0;

The same code pattern as above is found at the end of withdrawableCollateral from PooledCreditLine. https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L829-L832

The same approach can be used for repay from PooledCreditLine. The existing code is https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L987-L992

if (_amount >= _totalCurrentDebt) {
    _amount = _totalCurrentDebt;
    emit CompletePooledCreditLineRepaid(_id, msg.sender, _amount);
} else {
    emit PartialPooledCreditLineRepaid(_id, msg.sender, _amount);
}

The same code with a simple comparison is

if (_amount < _totalCurrentDebt) {
    emit PartialPooledCreditLineRepaid(_id, msg.sender, _amount);
} else {
    _amount = _totalCurrentDebt;
    emit CompletePooledCreditLineRepaid(_id, msg.sender, _amount);
}

Tools Used

Manual analysis

Recommended Mitigation Steps

Replace the comparison operator and reverse the logic to save gas using the suggestions above

2. Add payable to functions that won't receive ETH

Impact

Identifying a function as payable saves gas. Functions that have the onlyOwner modifier cannot be called by normal users and will not mistakenly receive ETH. These functions can be payable to save gas.

Proof of Concept

There are many functions that have the onlyOwner modifier and can be payable

LenderPool functions updateStartFeeFraction

PooledCreditLine functions updateBorrowLimitLimits updateIdealCollateralRatioLimits updateBorrowRateLimits updateCollectionPeriodLimits updateDurationLimits updateDefaultGracePeriodLimits updateGracePenaltyRateLimits updatePriceOracle updateSavingsAccount updateProtocolFeeFraction updateProtocolFeeCollector updateStrategyRegistry updateVerification terminate

Tools Used

Manual analysis

Recommended Mitigation Steps

Add payable to these functions for gas savings

3. Use calldata instead of memory for function parameters

Impact

Use calldata instead of memory for function parameters. Having function arguments use calldata instead of memory can save gas.

Proof of Concept

Two function parameters use memory instead of calldata https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/LenderPool.sol#L665-L666

Tools Used

Manual analysis

Recommended Mitigation Steps

Change function arguments from memory to calldata

4. Split up require statements instead of &&

Impact

Combining require statement conditions with && logic uses unnecessary gas. It is better to split up each part of the logical statement into a separate require statements

Proof of Concept

This pattern is found throughout PooledCreditLine https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L406 https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L418 https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L430 https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L442 https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L454 https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L466 https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L478 https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L642

Tools Used

Manual analysis

Recommended Mitigation Steps

Use separate require statements instead of concatenating with &&

5. Use newer solidity version

Impact

Solidity 0.7.6 is used in these Sublime contracts. Never versions of solidity include gas optimizations.

Proof of Concept

One example from solidity 0.8.10 is quoted from https://blog.soliditylang.org/2021/11/09/solidity-0.8.10-release-announcement/#full-changelog

Code Generator: Skip existence check for external contract if return data is expected. In this case, the ABI decoder will revert if the contract does not exist

The request function is one example of where this gas savings would apply https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L638

Tools Used

Manual analysis

Recommended Mitigation Steps

Use a newer solidity release