Sublime contest - Meta0xNull's results

Lines of code

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

Vulnerability details

Impact

IERC20(_borrowAsset).transfer(_to, _fee);

If the USDT token is supported as _borrowAsset, the unsafe version of .transfer(_to, _fee) may revert as there is no return value in the USDT token contract’s transfer() implementation (but the IERC20 interface expects a return value).

Function start() will break when _borrowAsset is USDT or Non ERC20 Compliant Tokens. USDT is one of the most borrowed Asset in DEFI. This may cause losing a lot of potential users.

Proof of Concept

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

Recommended Mitigation Steps

Use .safeTransfer instead of .transfer

IERC20(_borrowAsset).safeTransfer(_to, _fee);

1) function _accept() Use UnSafe and Deprecated safeApprove

Risk Rating: Low

Proof of Concept

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

Recommended

The OpenZeppelin SafeERC20 safeApprove() function has been deprecated. Using this deprecated function can lead to unintended reverts and potentially the locking of funds. Discussion: https://github.com/OpenZeppelin/openzeppelin-contracts/issues/2219

As suggested by the OpenZeppelin comment, replace safeApprove() with safeIncreaseAllowance().

2) Suggest function liquidate() Open to Public Rather than Lenders Only

Risk Rating: Informational

Proof of Concept

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

Recommended

@dev only one of the lenders can liquidate their pooled credit line

Is Impossible All Lenders know how to monitor their pooled credit line and call function liquidate(). Suggest Open function liquidate() to Public and so Bot Developer can built bot to monitor pooled credit line and call function liquidate() when needed.

3) registerSelf() Incorrect @dev note

Risk Rating: Informational

Proof of Concept

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/Verification/twitterVerifier.sol#L105

Recommended

The @dev note in registerSelf() mention "@dev only owner can register users" but the function actually allow Users to register themselve. Suggest change to "@dev users themselves can register themself".

4) updateVerification() Lack of Zero Address Check

Risk Rating: Low

Proof of Concept

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/Verification/twitterVerifier.sol#L189-L195

Recommended

require(_verification != address(0), "Address Can't Be Zero")

5) updateSignerAddress() Lack of Zero Address Check

Risk Rating: Low

Proof of Concept

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/Verification/twitterVerifier.sol#L203-L209

Recommended

require(_signerAddress != address(0), "Address Can't Be Zero")

6) Spelling Mistake "idenitifer"

Risk Rating: Informational

Proof of Concept

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

Recommended

There are multiple Spelling Mistake "idenitifer" in LenderPool.sol and PooledCreditLine.sol.

The correct spelling should be "identifier". Suggest use Find & Replace to find "idenitifer" and replace as "identifier".

1) Use bytes32 Rather Than String in struct UserStructData

Proof of Concept

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/Verification/twitterVerifier.sol#L36-L38

Recommended

Twitter ID Example: 10765432100123456789 Source: https://developer.twitter.com/en/docs/twitter-ids

Tweet ID Example: 40775075934244864 Source: https://groups.google.com/g/twitter-development-talk/c/4CGABcWH04E?pli=1

Both Twitter ID and Tweet ID above can fit into 32 bytes, then you should use bytes32 datatype rather than string as it is much cheaper in solidity. Basically, Any fixed size variable in solidity is cheaper than variable size. That will save gas on the contract.

2) Use bytes32 Rather Than String in twitterIdMap

Proof of Concept

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/Verification/twitterVerifier.sol#L48

Recommended

Same as Gas Report No.1 Above. Use bytes32 Rather Than String