Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Incorrect Remaining Points Calculation in closeBidTaker Allows Potential Over-claiming

cheatcode

Summary

The closeBidTaker function has a logical error in the calculation of userRemainingPoints. The function attempts to handle two different scenarios based on the offerSettleType, but there's a logical flaw in the calculation for the non-protected case.

Code

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L123

if (makerInfo.offerSettleType == OfferSettleType.Protected) {
offerInfo = preOfferInfo;
userRemainingPoints = stockInfo.points;
} else {
offerInfo = perMarkets.getOfferInfo(makerInfo.originOffer);
if (stockInfo.offer == address(0x0)) {
userRemainingPoints = stockInfo.points;
} else {
OfferInfo memory listOfferInfo = perMarkets.getOfferInfo(
stockInfo.offer
);
userRemainingPoints =
listOfferInfo.points -
listOfferInfo.usedPoints;
}
}

When stockInfo.offer is not zero (i.e., there's an active offer), it calculates userRemainingPoints based on the listOfferInfo, which is derived from stockInfo.offer. However, this calculation (listOfferInfo.points - listOfferInfo.usedPoints) doesn't take into account the actual remaining points for the user's stock. It's using the offer's points instead of the stock's points. This could lead to incorrect calculations of remaining points, potentially allowing users to claim more or fewer points than they should.

A more accurate approach would be to use stockInfo.points in both cases, as it directly represents the remaining points for the specific stock being closed.

Impact

Incorrect distribution of tokens when closing a bid taker position. If the remaining points are calculated incorrectly, users might receive more or fewer tokens than they should.

Mitigation

Modify the code to always use stockInfo.points for userRemainingPoints:

userRemainingPoints = stockInfo.points;

Poc

function test_closeBidTaker_incorrect_remaining_points() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Bid,
OfferSettleType.Turbo
)
);
address offerAddr = GenerateAddress.generateOfferAddress(0);
vm.stopPrank();
vm.startPrank(user2);
preMarktes.createTaker(offerAddr, 500);
address stockAddr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer(stockAddr, 0.015 * 1e18, 6000);
address newOfferAddr = GenerateAddress.generateOfferAddress(1);
vm.stopPrank();
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
uint256 user2BalanceBefore = mockUSDCToken.balanceOf(user2);
vm.prank(user2);
deliveryPlace.closeBidTaker(stockAddr);
uint256 user2BalanceAfter = mockUSDCToken.balanceOf(user2);
uint256 actualDifference = user2BalanceAfter - user2BalanceBefore;
uint256 expectedDifference = (500 * 0.01 * 1e18) / 1000; // Based on original 500 points
assertEq(actualDifference, expectedDifference, "Incorrect amount returned to user");
(,,,uint256 remainingPoints) = preMarktes.getStockInfo(stockAddr);
assertEq(remainingPoints, 500, "Incorrect remaining points in stock");
}
Updates

Lead Judging Commences

0xnevi Lead Judge
12 months ago
0xnevi Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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