L-01 : lower and upper bounds not checked during updating newFloor value
L-02 : function setNFTTypeValueETH() should not allow setting value for bytes32(0) NFTType
L-03 : validation check is very basic in function _validateRate()
L-04 : missing events in some important functions in NFTVault.sol
Details L-01
Title : lower and upper bounds not checked during updating newFloor value
If the protocol wants to override the oracle with a custom floor value, it will use the function overrideFloor(uint256 _newFloor)
However there is no input validation and the values can be set to any value.
To mitigate any misuse or error, there should be some lower and upper bound values defined, say wrt to the oracle price.
Proof of Concept
Contract : NFTVault.sol
Function : overrideFloor(uint256 _newFloor)
Recommended Mitigation Steps
One solution is to take the oracle price as reference and check if the newFloor value is within some range, like
Title : function setNFTTypeValueETH() should not allow setting value for bytes32(0) NFTType
The function setNFTTypeValueETH() can be used to set the value for any NFTType,
nftTypeValueETH[_type] = _amountETH;
Similarly the overrideFloor(uint256 _newFloor) is also used to set set the value
nftTypeValueETH[bytes32(0)] = _newFloor;
The cause of concern is that once a value is set for bytes32(0) using overrideFloor function, the value of bytes32(0) can be changed
via setNFTTypeValueETH() function also, which should be prohibited.
Proof of Concept
Contract : NFTVault.sol
Function : setNFTTypeValueETH(bytes32 _type, uint256 _amountETH)
Recommended Mitigation Steps
Add a check in setNFTTypeValueETH() for filtering out values of _type = bytes32(0)
require(_type != bytes32(0), "cannot update value for type = 0");
Details L-03
Title : validation check is very basic in function _validateRate()
_validationRate() does a very basic check for denominator >0 and denominator > numerator so that the variable value is not > 1.
However there needs to be additional check so as to prevent rugpull, malicious activity or changes by mistake.
Example, currently its possible to set the organizationFeeRate to 100%, the debtInterestApr to 100% or the insurancePurchaseRate to 100%
There should be additional check for the values to be within some max values.
This will give some confidence to the users, and protection against some malicious events.
Proof of Concept
Contract : NFTVault.sol
Function : _validateRate()
Recommended Mitigation Steps
Define MAX values for each of the rate, and check them additionally during change.
Details L-04
Title : missing events in some important functions in NFTVault.sol
There are some important functions, like below,
line#232 : function setCreditLimitRate(Rate memory _creditLimitRate)
line#247 : function setLiquidationLimitRate(Rate memory _liquidationLimitRate)
These should ideally generate events.
Recommended Mitigation Steps
Add relevant events during changing important values.
Summary of Findings for Low / Non-Critical issues
Details L-01
Title : lower and upper bounds not checked during updating newFloor value
If the protocol wants to override the oracle with a custom floor value, it will use the function overrideFloor(uint256 _newFloor) However there is no input validation and the values can be set to any value. To mitigate any misuse or error, there should be some lower and upper bound values defined, say wrt to the oracle price.
Proof of Concept
Contract : NFTVault.sol
Function : overrideFloor(uint256 _newFloor)
Recommended Mitigation Steps
One solution is to take the oracle price as reference and check if the newFloor value is within some range, like
Details L-02
Title : function setNFTTypeValueETH() should not allow setting value for bytes32(0) NFTType
The function setNFTTypeValueETH() can be used to set the value for any NFTType,
Similarly the overrideFloor(uint256 _newFloor) is also used to set set the value
The cause of concern is that once a value is set for bytes32(0) using overrideFloor function, the value of bytes32(0) can be changed via setNFTTypeValueETH() function also, which should be prohibited.
Proof of Concept
Contract : NFTVault.sol Function : setNFTTypeValueETH(bytes32 _type, uint256 _amountETH)
Recommended Mitigation Steps
Add a check in setNFTTypeValueETH() for filtering out values of _type = bytes32(0)
Details L-03
Title : validation check is very basic in function _validateRate()
_validationRate() does a very basic check for denominator >0 and denominator > numerator so that the variable value is not > 1. However there needs to be additional check so as to prevent rugpull, malicious activity or changes by mistake. Example, currently its possible to set the organizationFeeRate to 100%, the debtInterestApr to 100% or the insurancePurchaseRate to 100%
There should be additional check for the values to be within some max values. This will give some confidence to the users, and protection against some malicious events.
Proof of Concept
Contract : NFTVault.sol Function : _validateRate()
Recommended Mitigation Steps
Define MAX values for each of the rate, and check them additionally during change.
Details L-04
Title : missing events in some important functions in NFTVault.sol
There are some important functions, like below, line#232 : function setCreditLimitRate(Rate memory _creditLimitRate) line#247 : function setLiquidationLimitRate(Rate memory _liquidationLimitRate)
These should ideally generate events.
Recommended Mitigation Steps
Add relevant events during changing important values.