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

User receive `pointsToken` as well his `collateralToken` back when he close his bidTaker ie deliveryPlace:closeBidTaker()

0xaraj

Summary

User receive pointsToken as well his collateralToken back when he close his bidTaker ie deliveryPlace:closeBidTaker() due to wrong calculation of userCollateralFee in closeBidTaker()

Vulnerability Details

Buyer receives pointsToken when they close their bidTaker using deliveryPlace:closeBidTaker(). If offerOwner settled the offer with less settledPoints than offerInfo.usedPoints then buyer receives the remainingCash ie collateralToken

function closeBidTaker(address _stock) external {
...
uint256 collateralFee;
@> if (offerInfo.usedPoints > offerInfo.settledPoints) {
if (offerInfo.offerStatus == OfferStatus.Virgin) {
collateralFee = 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);
​
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType, offerInfo.collateralRate, usedAmount, true, Math.Rounding.Floor
);
}
}
​
@> uint256 userCollateralFee = collateralFee.mulDiv(userRemainingPoints, offerInfo.usedPoints, Math.Rounding.Floor);
​
tokenManager.addTokenBalance(
@> TokenBalanceType.RemainingCash, _msgSender(), makerInfo.tokenAddress, userCollateralFee
);
...
}

Now the problem is with the calculation of userCollateralFee. As result, user receives the pointsToken as well all his collateralToken back

//Here is how this works(simple example)

  1. Suppose a user1 created an ask offer using createOffer() for 1000 points with 1000e18 amount(collateralToken) at 10000(100%) collateralRate, which means user1 will pay 1000e18 collateralToken to capitalPool

  2. User2 bought all 1000 points paying 1000e18(buyAmount) + tradeTax + platformFee

  3. Owner updated the marketPlace with tokenPerPoints = 1e18

  4. User1 settled the offer using settleAskMaker() with only 500 settledPoints instead of 1000 settledPoints

  5. User2 close his bidTaker using closeBidTaker(), which calculates the remainingCash because settledPoints are less than usedPoints

  6. User2 should receive 500 pointsToken(500 settlePoints * 1e18 tokenPerPoints) & 500e18 collateralToken as remainingCash because he got only half of the pointsToken ie 500 pointsToken instead of 1000 pointsToken.

  7. But user2 receives full 1000e18 collateralToken(which he paid to buy 1000 points) along with 500 pointsToken

//Here is coded PoC, which clearly shows above situation

function test_BuyerReceivesPointsTokenWithFullCollateral() public {
//User created ask offer
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 1000e18, 10000, 300, OfferType.Ask, OfferSettleType.Turbo
)
);
vm.stopPrank();
​
//User2 bought all points
vm.prank(user2);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 1000);
​
//Owner updated the marketPlace with tokenPerPoint = 1e18
vm.prank(user1);
systemConfig.updateMarket("Backpack", address(mockPointToken), 1e18, block.timestamp - 1, 3600);
​
//User settled the ask offer with only 500 points
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
deliveryPlace.settleAskMaker(offerAddr, 500);
vm.stopPrank();
​
//User2 closed the bid taker
address stock1Addr = GenerateAddress.generateStockAddress(1);
vm.prank(user2);
deliveryPlace.closeBidTaker(stock1Addr);
​
//RemainingCash of user2 is 1000e18 collateralToken instead of only 500e18
uint256 remainingCash =
tokenManager.userTokenBalanceMap(user2, address(mockUSDCToken), TokenBalanceType.RemainingCash);
assertEq(remainingCash, 1000e18);
​
//PointsToken is 500e18 ie 500 pointsToken
uint256 pointsToken =
tokenManager.userTokenBalanceMap(user2, address(mockPointToken), TokenBalanceType.PointToken);
assertEq(pointsToken, 500e18);
}

Impact

Buyer receives full collateralToken as well as pointsToken

Tools Used

Manual Review

Recommendations

Rewrite the userCollateralFee which should consider the pointsToken that buyer received & subtract the equivalent amount of collateralToken from the buyer's amount

Updates

Lead Judging Commences

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

finding-PreMarkets-partial-closeBidTaker-userCollateralFee-wrong-partial

Valid High, afaik, partial settlements are a valid flow and so when closing bid offers by takers and/or when settling offers by makers, we should return a proportionate amount of funds based on points settled. This issues could be related to issue #1008, but seems to be describing a different issue.

Appeal created

cryptomoon Auditor
about 1 year ago
0xbrivan2 Auditor
about 1 year ago
0xnevi Lead Judge
about 1 year ago
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.