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

User will lose their collaterals when they aborts bid taker

h2134

Summary

User will lose their collaterals when they aborts bid taker, due to the incorrect calculation of refunding amount.

Vulnerability Details

When user buys an ask offer from a maker, they need to deposit collaterals to create taker. Later if the maker abort the offer, the user can also abort the taker by calls abortBidTaker() and the user's collaterals will be refunded.

tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
_msgSender(),
makerInfo.tokenAddress,
@> transferAmount
);

Unfortunately, the transferAmount is not correctly calculated.

uint256 depositAmount = stockInfo.points.mulDiv(
preOfferInfo.points,
preOfferInfo.amount,
Math.Rounding.Floor
);
​
uint256 transferAmount = OfferLibraries.getDepositAmount(
preOfferInfo.offerType,
preOfferInfo.collateralRate,
depositAmount,
false,
Math.Rounding.Floor
);

transferAmount is calculated based on depositAmount, and depositAmount is calculated as stockInfo.points * preOfferInfo.points / preOfferInfo.amount. Suppose stockInfo.points is , preOfferInfo.points is , the depositAmount will be , resulting in transferAmount is and user will not receive anying refunding.

Please copy the PoC code into PreMarkets.t.sol to verify:

function testAudit_UserLossesCollateralsWhenAbortBidTaker() public {
// Create Market
address auditMarketPlace = GenerateAddress.generateMarketPlaceAddress("AuditMarket");
vm.startPrank(user1);
systemConfig.createMarketPlace("AuditMarket", false);
vm.stopPrank();
​
address alice = makeAddr("Alice");
deal(address(mockUSDCToken), alice, 1200e18);
​
address bob = makeAddr("Bob");
deal(address(mockUSDCToken), bob, 103.5e18);
​
uint256 offerId = preMarktes.offerId();
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);
​
// Alice creates an ask offer
vm.startPrank(alice);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createOffer(CreateOfferParams({
marketPlace: auditMarketPlace,
tokenAddress: address(mockUSDCToken),
points: 1000,
amount: 1000e18,
collateralRate: 12000,
eachTradeTax: 300,
offerType: OfferType.Ask,
offerSettleType: OfferSettleType.Turbo
}));
vm.stopPrank();
​
// Bob creates taker
vm.startPrank(bob);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 100);
vm.stopPrank();
​
// Alice aborts offer
vm.prank(alice);
preMarktes.abortAskOffer(stockAddr, offerAddr);
​
// Bob aborts taker
address bobStockAddr = GenerateAddress.generateStockAddress(preMarktes.offerId() - 1);
StockInfo memory stockInfo = preMarktes.getStockInfo(bobStockAddr);
vm.prank(bob);
preMarktes.abortBidTaker(bobStockAddr, stockInfo.preOffer);
​
// Bob receives no refund
assertEq(tokenManager.userTokenBalanceMap(bob, address(mockUSDCToken), TokenBalanceType.MakerRefund), 0);
}

Impact

User loses all their collaterals.

Tools Used

Manual Review

Recommendations

The correct calculation of `depositAmount` should be:

uint256 depositAmount = stockInfo.points.mulDiv(
- preOfferInfo.points,
- preOfferInfo.amount,
+ preOfferInfo.amount,
+ preOfferInfo.points,
Math.Rounding.Floor
);
Updates

Lead Judging Commences

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

finding-DeliveryPlace-settleAskTaker-closeBidTaker-wrong-makerinfo-token-address-addToken-balance

Valid high severity, In `settleAskTaker/closeBidTaker`, by assigning collateral token to user balance instead of point token, if collateral token is worth more than point, this can cause stealing of other users collateral tokens within the CapitalPool contract, If the opposite occurs, user loses funds based on the points they are supposed to receive

Support

FAQs

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