search

code-423n4 / 2022-04-jpegd-findings

1 stars 1 forks source link

missing lowerbounds for critical parameters creditLimitRate & liquidationLimitRate. #159

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/NFTVault.sol#L230-L259

Vulnerability details

The protocol allows the DAO to change the critical parameters via setCreditLimitRate and setLiquidationLimitRate. However there is no check on the lower bounds, so if the current rates are 32/100 and 33/100 for the creditLimitRate & liquidationLimitRate respectively, and there is a change effected by DAO, say to 1/100 and 2/100 for creditLimitRate & liquidationLimitRate, then effectively all the borrow positions can be liquidated.

Impact

The code in its current form permits for a possible rug pull by setCreditLimitRate([1, 100]), followed by setLiquidationLimitRate([2,100]) and liquidate() functions. The likelihood that this would be exploited intentionally by the DAO or happen unintentionally is very low, but not impossible, but If exploited, the impact would be huge. Hence marking this a medium severity issue.

Proof of Concept

Contract : vaults/NFTVault.sol Functions : setCreditLimitRate and setLiquidationLimitRate

Recommended Mitigation Steps

Define upper and lower bound values as MIN and MAX for both these critical parameters, and add a check while changing in the respective set functions.

spaghettieth commented 2 years ago

From the contest's readme:

Wardens can assume that governance variables are set appropriately, and can reach out on Discord for any further clarification.
dmvt commented 2 years ago

Out of scope