ritik99
commented
2 years ago The issues mentioned by the warden are relevant
Open code423n4 opened 2 years ago
ritik99
commented
2 years ago The issues mentioned by the warden are relevant
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