pkqs90 - `STANDARD_EXPANSION_FACTOR` is not needed - required collateral amount is 1e18 times smaller than it should be.
Closed this issue · 0 comments
pkqs90
high
STANDARD_EXPANSION_FACTOR
is not needed - required collateral amount is 1e18 times smaller than it should be.
Summary
When calculating the requiredCollateral for a loan, it is multipled by STANDARD_EXPANSION_FACTOR
(1e18). This is not required, and the current implementation incorrectly allows 1e18 times smaller collateral amount to create a loan.
Vulnerability Detail
In acceptFundsForAcceptBid
, the _collateralAmount
is expanded by STANDARD_EXPANSION_FACTOR
(1e18) when comparing with requiredCollateral
. This is incorrect.
By reading the code comments, it says that _getPriceFromSqrtX96()
function returns a price ratio expanded by 1e18, which is incorrect. What this function does is calculating the _sqrtPriceX96 * _sqrtPriceX96 / 2**96
, which is the actualPrice * 2**96
. The 2**96
already serves as the precision factor, and the 1e18 is simply incorrect.
function acceptFundsForAcceptBid(
address _borrower,
uint256 _bidId,
uint256 _principalAmount,
uint256 _collateralAmount,
address _collateralTokenAddress,
uint256 _collateralTokenId,
uint32 _loanDuration,
uint16 _interestRate
) external onlySmartCommitmentForwarder whenNotPaused {
require(
_collateralTokenAddress == address(collateralToken),
"Mismatching collateral token"
);
//the interest rate must be at least as high has the commitment demands. The borrower can use a higher interest rate although that would not be beneficial to the borrower.
require(_interestRate >= getMinInterestRate(), "Invalid interest rate");
//the loan duration must be less than the commitment max loan duration. The lender who made the commitment expects the money to be returned before this window.
require(_loanDuration <= maxLoanDuration, "Invalid loan max duration");
require(
getPrincipalAmountAvailableToBorrow() >= _principalAmount,
"Invalid loan max principal"
);
//this is expanded by 10**18
> uint256 requiredCollateral = getCollateralRequiredForPrincipalAmount(
> _principalAmount
> );
require(
(_collateralAmount * STANDARD_EXPANSION_FACTOR) >=
requiredCollateral,
"Insufficient Borrower Collateral"
);
...
}
//this result is expanded by UNISWAP_EXPANSION_FACTOR
function _getUniswapV3TokenPairPrice(uint32 _twapInterval)
internal
view
returns (uint256)
{
// represents the square root of the price of token1 in terms of token0
uint160 sqrtPriceX96 = getSqrtTwapX96(_twapInterval);
//this output is the price ratio expanded by 1e18
> return _getPriceFromSqrtX96(sqrtPriceX96);
}
//this result is expanded by UNISWAP_EXPANSION_FACTOR
function _getPriceFromSqrtX96(uint160 _sqrtPriceX96)
internal
pure
returns (uint256 price_)
{
uint256 priceX96 = (uint256(_sqrtPriceX96) * uint256(_sqrtPriceX96)) /
(2**96);
// sqrtPrice is in X96 format so we scale it down to get the price
// Also note that this price is a relative price between the two tokens in the pool
// It's not a USD price
price_ = priceX96;
}
// ---- TWAP
function getSqrtTwapX96(uint32 twapInterval)
public
view
returns (uint160 sqrtPriceX96)
{
if (twapInterval == 0) {
// return the current price if twapInterval == 0
(sqrtPriceX96, , , , , , ) = IUniswapV3Pool(UNISWAP_V3_POOL)
.slot0();
} else {
uint32[] memory secondsAgos = new uint32[](2);
secondsAgos[0] = twapInterval; // from (before)
secondsAgos[1] = 0; // to (now)
(int56[] memory tickCumulatives, ) = IUniswapV3Pool(UNISWAP_V3_POOL)
.observe(secondsAgos);
// tick(imprecise as it's an integer) to price
sqrtPriceX96 = TickMath.getSqrtRatioAtTick(
int24(
(tickCumulatives[1] - tickCumulatives[0]) /
int32(twapInterval)
)
);
}
}
Impact
Allows borrowers to borrow a loan with 1e18 times smaller of required collateral amount.
Code Snippet
Tool used
Manual review
Recommendation
Remove STANDARD_EXPANSION_FACTOR
.
Duplicate of #58