Summary
The settleAskMaker function is designed to facilitate the transfer of the points tokens after the admin has set the Token Generation Event (TGE) parameters for settlement. The function aims to transfer the agreed-upon points tokens from the Maker to the relevant party as per the accepted points by the Taker.
However, an issue has been identified in the function related to the calculation of the settledPointTokenAmount. Specifically, the function multiplies tokenPerPoint by settledPoint to derive the settledPointTokenAmount. This calculation is unnecessary and potentially flawed because the number of points has already been determined and accepted by the Taker. Additionally, since the points may be in the form of ERC20 tokens, the only additional consideration necessary is the decimals associated with point tokens.
Vulnerability Details
The function performs an unnecessary calculation to derive settledPointTokenAmount by multiplying tokenPerPoint and settledPoint. Since the points were already accepted by the Taker, this calculation introduces an unnecessary step that could lead to errors or inconsistencies, particularly if the points are represented as ERC20 tokens.
The points could be represented as ERC20 tokens, which means handling decimals correctly is crucial. The current implementation does not take into account the need for appropriate handling of decimals when transferring tokens, which could result in incorrect token amounts being transferred.
The core problem is that settledPointTokenAmountis being used as the points to be transferred to the Taker.
This issue and its effect can also be found in settleAskTaker function and closeBidTaker function.
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
);
}
POC
Run test on test/PreMarkets.t.sol
function test_wrongPointTransferred() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 2000 * 1e18, 12000, 300, OfferType.Ask, OfferSettleType.Turbo
)
);
vm.stopPrank();
vm.startPrank(user3);
address offerAddr = GenerateAddress.generateOfferAddress(0);
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();
uint256 capitalPoolBalance = mockPointToken.balanceOf(address(capitalPool));
assert(capitalPoolBalance == 1000e18);
}
Impact
Incorrect Token Transfer: The Maker could end up transferring more or fewer tokens than agreed, leading to potential financial loss or disputes.
Contract Discrepancy: The discrepancy between expected and actual token transfers could undermine the integrity of the contract and its intended operation.
Tools Used
Manual Review
Recommendations
In the settleAskMaker function, transfer the correct points token.
