Summary
The settleAskMaker function is responsible for updating various parameters of an offer after the Maker initiates settlement. This includes updating settledPoints, settledPointTokenAmount, and offerStatus. However, a critical issue has been identified: the settledCollateralAmount is not updated within this function, despite being an essential component of the offer’s financial details. This oversight could lead to inconsistencies in the contract’s data, resulting in financial discrepancies, contract misbehavior, and potential exploitation.
Vulnerability Details
When the settleAskMaker function is called, it updates the offer’s settledPoints, settledPointTokenAmount, and offerStatus. However, the settledCollateralAmount—which should reflect the amount of collateral that has been settled—is not updated. This omission leaves the offer’s financial data incomplete and inaccurate.
The offer’s data should accurately reflect all aspects of the settlement, including the collateral. The failure to update settledCollateralAmount creates a discrepancy between the actual collateral settled and the data recorded in the contract.
function settleAskMaker(address _offer, uint256 _settledPoints) external {
(
OfferInfo memory offerInfo,
MakerInfo memory makerInfo,
MarketPlaceInfo memory marketPlaceInfo,
MarketPlaceStatus status
) = getOfferInfo(_offer);
if (_settledPoints > offerInfo.usedPoints) {
revert InvalidPoints();
}
if (marketPlaceInfo.fixedratio) {
revert FixedRatioUnsupported();
}
if (offerInfo.offerType == OfferType.Bid) {
revert InvalidOfferType(OfferType.Ask, OfferType.Bid);
}
if (offerInfo.offerStatus != OfferStatus.Virgin && offerInfo.offerStatus != OfferStatus.Canceled) {
revert InvalidOfferStatus();
}
if (status == MarketPlaceStatus.AskSettling) {
if (_msgSender() != offerInfo.authority) {
revert Errors.Unauthorized();
}
} else {
if (_msgSender() != owner()) {
revert Errors.Unauthorized();
}
if (_settledPoints > 0) {
revert InvalidPoints();
}
}
uint256 settledPointTokenAmount = marketPlaceInfo.tokenPerPoint * _settledPoints;
ITokenManager tokenManager = tadleFactory.getTokenManager();
if (settledPointTokenAmount > 0) {
tokenManager.tillIn(_msgSender(), marketPlaceInfo.tokenAddress, settledPointTokenAmount, true);
}
uint256 makerRefundAmount;
if (_settledPoints == offerInfo.usedPoints) {
if (offerInfo.offerStatus == OfferStatus.Virgin) {
makerRefundAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType, offerInfo.collateralRate, offerInfo.amount, true, Math.Rounding.Floor
);
} else {
uint256 usedAmount =
offerInfo.amount.mulDiv(offerInfo.usedPoints, offerInfo.points, Math.Rounding.Floor);
makerRefundAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType, offerInfo.collateralRate, usedAmount, true, Math.Rounding.Floor
);
}
tokenManager.addTokenBalance(
TokenBalanceType.SalesRevenue, _msgSender(), makerInfo.tokenAddress, makerRefundAmount
);
}
IPerMarkets perMarkets = tadleFactory.getPerMarkets();
@> perMarkets.settledAskOffer(_offer, _settledPoints, settledPointTokenAmount);
emit SettleAskMaker(
makerInfo.marketPlace,
offerInfo.maker,
_offer,
_msgSender(),
_settledPoints,
settledPointTokenAmount,
makerRefundAmount
);
}
function settledAskOffer(address _offer, uint256 _settledPoints, uint256 _settledPointTokenAmount)
external
onlyDeliveryPlace(tadleFactory, _msgSender())
{
OfferInfo storage offerInfo = offerInfoMap[_offer];
offerInfo.settledPoints = _settledPoints;
offerInfo.settledPointTokenAmount = _settledPointTokenAmount;
offerInfo.offerStatus = OfferStatus.Settled;
emit SettledAskOffer(_offer, _settledPoints, _settledPointTokenAmount);
}
POC
Run the test on test/PreMarkets.t.sol
function test_settledAmount() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 2000 * 1e18, 12000, 300, OfferType.Ask, OfferSettleType.Turbo
)
);
vm.stopPrank();
address offerAddr = GenerateAddress.generateOfferAddress(0);
vm.startPrank(user3);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 1000);
vm.stopPrank();
vm.prank(user1);
systemConfig.updateMarket("Backpack", address(mockPointToken), 2 * 1e18, block.timestamp - 1, 3600);
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 2000 * 10 ** 18);
deliveryPlace.settleAskMaker(offerAddr, 1000);
vm.stopPrank();
vm.startPrank(user3);
address stock2Addr = GenerateAddress.generateStockAddress(1);
deliveryPlace.closeBidTaker(stock2Addr);
vm.stopPrank();
OfferInfo memory info = preMarktes.getOfferInfo(offerAddr);
assert(info.settledCollateralAmount > 0);
}
Impact
The absence of an updated settledCollateralAmount can lead to inconsistencies between the actual state of the offer and the recorded data.
This inconsistency can cause confusion and errors in the contract’s logic. For example, functions that depend on the correct collateral amount being recorded may behave unpredictably, leading to incorrect calculations.
Tools Used
Manual Review
Recommendations
Update the settledCollateralAmount
- function settledAskOffer(address _offer, uint256 _settledPoints, uint256 _settledPointTokenAmount)
+ function settledAskOffer(address _offer, uint256 _settledPoints, uint256 settledCollateralAmount, uint256 _settledPointTokenAmount)
external
onlyDeliveryPlace(tadleFactory, _msgSender())
{
// settledCollateralAmount is not updated
OfferInfo storage offerInfo = offerInfoMap[_offer];
offerInfo.settledPoints = _settledPoints;
offerInfo.settledPointTokenAmount = _settledPointTokenAmount;
offerInfo.offerStatus = OfferStatus.Settled;
+ offerInfo.settledCollateralAmount = settledCollateralAmount
emit SettledAskOffer(_offer, _settledPoints, _settledPointTokenAmount);
}
