Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

`PreMarkets` Reports Incorrect Deposits To `TokenManager`

auditweiler

Summary

When a collateralRate is greater than COLLATERAL_RATE_DECIMAL_SCALER, PreMarkets can amplify the total number of tokens pulled from the caller by increasing the inbound depositAmount. However, only the original unamplified depositAmount is actually accounted for, even though more tokens than this have been transferred from the caller.

Vulnerability Details

When creating an offer, the caller must specify at a minimum a collateralRate of COLLATERAL_RATE_DECIMAL_SCALER, but this value can be arbitrarily larger:

if (params.collateralRate < Constants.COLLATERAL_RATE_DECIMAL_SCALER) {
revert InvalidCollateralRate();
}

During calls to createTaker(address,uint256), the inbound depositAmount is explicitly amplified by this amount via _depositTokenWhenCreateTaker when the collateralRate is over COLLATERAL_RATE_DECIMAL_SCALER via the underlying call to getDepositAmount:

function getDepositAmount(
OfferType _offerType,
uint256 _collateralRate,
uint256 _amount,
bool _isMaker,
Math.Rounding _rounding
) internal pure returns (uint256) {
/// @dev bid offer
if (_offerType == OfferType.Bid && _isMaker) {
return _amount;
}
/// @dev ask order
if (_offerType == OfferType.Ask && !_isMaker) {
return _amount;
}
return
Math.mulDiv(
_amount,
_collateralRate,
Constants.COLLATERAL_RATE_DECIMAL_SCALER,
_rounding
); /// @audit depositAmount_can_be_increased
}

It is this amplified amount which is actually charged to the caller:

uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
depositAmount,
false,
Math.Rounding.Ceil
); /// @audit depositAmount_can_be_increased
transferAmount = transferAmount + platformFee + tradeTax;
tokenManager.tillIn{value: msg.value}(
_msgSender(),
makerInfo.tokenAddress,
transferAmount,
false
); /// @audit inflated_depositAmount_charged

However, when crediting account balances in _updateTokenBalanceWhenCreateTaker, only the original unscaled depositAmount is actually accounted for:

_updateTokenBalanceWhenCreateTaker(
_offer,
tradeTax,
depositAmount, /// @audit original_unscaled_depositAmount
offerInfo,
makerInfo,
tokenManager
);
/// @dev update sales revenue
if (offerInfo.offerType == OfferType.Ask) {
tokenManager.addTokenBalance(
TokenBalanceType.SalesRevenue,
offerInfo.authority,
makerInfo.tokenAddress,
_depositAmount /// @audit credits_unscaled_depositAmount
);
} else {
tokenManager.addTokenBalance(
TokenBalanceType.SalesRevenue,
_msgSender(),
makerInfo.tokenAddress,
_depositAmount /// @audit credits_unscaled_depositAmount
);
}

Consequently, although we may pull expressly more tokens from the caller than the depositAmount, these excess tokens are not actually accounted for.

Impact

Claims to token deposits are devalued when an offer's collateralRate is greater than COLLATERAL_RATE_DECIMAL_SCALER, resulting in an accounting imbalance which impacts upon rightful claims to the accrued capital.

This eventually materializes as stuck tokens.

Tools Used

Manual Review

Recommendations

To prevent the devaluation of token balances, instead of crediting depositAmount in sales revenue, we should be crediting result of the call to getDepositAmount:

/// @notice Pass the correct amount.
/// @dev This is done for demonstrative purposes only. Rather
/// @dev than recomputing the amount here, it would be more
/// @dev efficient and easier to reason about by returning
/// @dev the evaluated `transferAmount` from the internal
/// @dev call to `_depositTokenWhenCreateTaker` and passing this
/// @dev as a parameter into `_updateTokenBalanceWhenCreateTaker`.
_depositAmount = OfferLibraries.getDepositAmount(...);
​
/// @dev update sales revenue
if (offerInfo.offerType == OfferType.Ask) {
tokenManager.addTokenBalance(
TokenBalanceType.SalesRevenue,
offerInfo.authority,
makerInfo.tokenAddress,
_depositAmount
);
} else {
tokenManager.addTokenBalance(
TokenBalanceType.SalesRevenue,
_msgSender(),
makerInfo.tokenAddress,
_depositAmount
);
}
Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

[invalid] finding-PreMarkets-collateralRate-unupdated-balance

Valid high, the additional collateral based on collateralRate is not updated to taker balance for protected mode. This results in incorrect collateral refunded to taker during settlement.

Appeal created

0xnevi Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[invalid] finding-PreMarkets-collateralRate-unupdated-balance

Valid high, the additional collateral based on collateralRate is not updated to taker balance for protected mode. This results in incorrect collateral refunded to taker during settlement.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.