Sublime contest - BouSalman's results

QA Report for sublime protocol

This report do list low-risk and non-critical findings presented in the sublime protocol codebase, these findings do not effect any assets connected to users or the protocol. However these issues are related to coding and security best practices.

Findings

unchecked on ERC20 transfer functions

The transfer() and transferFrom() functions return a Boolean value which should be checked for successful transfer, Some tokens do not revert if the transfer did fail but return false. the protocol sublime do have some lines for code that make a use of these functions and do not check the return value, this may effect the logic of the code and should be checked before continue processing transactions.

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

Usage of Deprecated Library Functions

Avoid using Deprecated Library Functions, for example in the sublime codebase there is a use of Aprove() and safeAprove() functions which is discouraged. it's advised to use the safeIncreaseAllowance() function.

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

Unused imported files

There are some unused imports in the protocol, this would decrease code quality and effect code audit time. it's advised to remove it before deploying the protocol.

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

Missing zero address checking

There is multiple instances inside the sublime protocol where the logic do not validate for zero address when assigning value, it's best practice to sanitize addresses before saving them in the state since there is a possibly to interrupt protocol business or loss of funds.

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/LenderPool.sol#L327 https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/LenderPool.sol#L406 https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/LenderPool.sol#L210-L212 https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L593-L594

Function type from public to external

Some of the implemented functions inside the protocol are of type Public, However these functions are not used within the contracts. functions like this should be labeled external to have a better code readability.

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

Unchecked return value of functions

there is functions declared inside the codebase of sublime that returns values to the caller, However No checking or handling for return values is done when calling these functions inside the contracts. it's advised to check for any returned values to make sure the business logic is followed correctly.

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

Missing Netspec comments for parameters

The codebase of sublime protocol is following the Netspec style when commenting and documenting the code, However there is instances where the Netspec comments is missing for function parameters. it's advised to include comments for all used parameters to have a matched code-docs.

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L718-L720 https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/Verification/twitterVerifier.sol#L78-L89

Unclear comments

comments in the code should be clear and obvious to help the maintainers of the code otherwise it will be confusing and waste resources. for example the following comment state number of seconds in an year however the code is calculating just number of days in a year.

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