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

Bid offer maker receives no refund when the offer is partially filled

h2134

Summary

Bid offer maker receives no refund when the offer is partially filled.

Vulnerability Details

When a maker creates a bid offer, they deposit collaterals for purchasing points.

{
/// @dev transfer collateral from _msgSender() to capital pool
uint256 transferAmount = OfferLibraries.getDepositAmount(
params.offerType,
params.collateralRate,
params.amount,
true,
Math.Rounding.Ceil
);
​
​
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.tillIn{value: msg.value}(
_msgSender(),
params.tokenAddress,
transferAmount,
false
);
}

The offer can be partially filled by a seller. When the offer is settled, the maker will receive point tokens on a pro rata basis. For example, Alice creates a bid offer, points is and amount is , Alice is required to deposit collateral tokens. If Bob fills the offer with points, then when the offer is settled, Alice will pay collaterals to receive 100e18 point tokens (suppose tokenPerPoint is ). However, protocol does not refund the remaining collateral tokens to Alice, Alice hence lose the funds.

Please follow the steps to run the PoC:

  • Change Line 361 as below (this is to fix another issue in the code base: bid offer should be settled by the taker):

- if (_msgSender() != offerInfo.authority) {
+ if (_msgSender() != stockInfo.authority) {

  • Change Line 387 as below (this is to fix another issue in the codebase: maker should receive point token instead of collateral token when the offer is settled):

- makerInfo.tokenAddress,
+ marketPlaceInfo.tokenAddress,

Run the PoC in PreMarkets.t.sol:

function testAudit_BidOfferMakerNotRefundedWhenTheOfferIsPartiallySettled() public {
// Create Market
address auditMarketPlace = GenerateAddress.generateMarketPlaceAddress("AuditMarket");
vm.startPrank(user1);
systemConfig.createMarketPlace("AuditMarket", false);
systemConfig.updateMarket("AuditMarket", address(mockPointToken), 1e18, block.timestamp + 30 days, 2 days);
vm.stopPrank();
​
address alice = makeAddr("Alice");
deal(address(mockUSDCToken), alice, 1000e18);
​
address bob = makeAddr("Bob");
deal(address(mockUSDCToken), bob, 123.5e18);
​
address aliceOfferAddr = GenerateAddress.generateOfferAddress(preMarktes.offerId());
​
// Alice creates an Bid 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: 0,
offerType: OfferType.Bid,
offerSettleType: OfferSettleType.Turbo
}));
vm.stopPrank();
​
address bobStockAddr = GenerateAddress.generateStockAddress(preMarktes.offerId());
​
// Bob creates taker
vm.startPrank(bob);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(aliceOfferAddr, 100);
vm.stopPrank();
​
// AskSettling
vm.warp(block.timestamp + 31 days);
deal(address(mockPointToken), bob, 100e18);
​
// Bob settles
vm.startPrank(bob);
mockPointToken.approve(address(tokenManager), type(uint256).max);
deliveryPlace.settleAskTaker(bobStockAddr, 100);
vm.stopPrank();
​
// Alice pays 100e18 collaterals for point tokens, the remaining collateral tokens are not refunded
assertEq(tokenManager.userTokenBalanceMap(alice, address(mockPointToken), TokenBalanceType.PointToken), 100e18);
assertEq(tokenManager.userTokenBalanceMap(alice, address(mockPointToken), TokenBalanceType.TaxIncome), 0);
assertEq(tokenManager.userTokenBalanceMap(alice, address(mockPointToken), TokenBalanceType.ReferralBonus), 0);
assertEq(tokenManager.userTokenBalanceMap(alice, address(mockPointToken), TokenBalanceType.SalesRevenue), 0);
assertEq(tokenManager.userTokenBalanceMap(alice, address(mockPointToken), TokenBalanceType.RemainingCash), 0);
assertEq(tokenManager.userTokenBalanceMap(alice, address(mockPointToken), TokenBalanceType.MakerRefund), 0);
​
assertEq(tokenManager.userTokenBalanceMap(alice, address(mockUSDCToken), TokenBalanceType.TaxIncome), 0);
assertEq(tokenManager.userTokenBalanceMap(alice, address(mockUSDCToken), TokenBalanceType.ReferralBonus), 0);
assertEq(tokenManager.userTokenBalanceMap(alice, address(mockUSDCToken), TokenBalanceType.SalesRevenue), 0);
assertEq(tokenManager.userTokenBalanceMap(alice, address(mockUSDCToken), TokenBalanceType.RemainingCash), 0);
assertEq(tokenManager.userTokenBalanceMap(alice, address(mockUSDCToken), TokenBalanceType.MakerRefund), 0);
assertEq(tokenManager.userTokenBalanceMap(alice, address(mockUSDCToken), TokenBalanceType.PointToken), 0);
}

Impact

User lose funds if their bid offer is partially settled.

Tools Used

Manual Review

Recommendations

When an bid offer is partially filled, the remaining collateral tokens should be refunded.

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.