Tadle

Tadle
DeFiFoundry
27,750 USDC
View results
Submission Details
Severity: high
Invalid

Unintended Points Transfer Causes Bid-Makers to Lose Funds in `DeliveryPlace::settleAskTaker`

Summary

Users who want to buy points must transfer points first

Vulnerability Details

when the bid-makers want to buy points in exchange for their token, they have to transfer them to the capitalPool to receive points. this is unintended and leads to loss of funds or the DeliveryPlace::settleAskTaker function not working properly.

function settleAskTaker(address _stock, uint256 _settledPoints) external {
// ...
uint256 settledPointTokenAmount = marketPlaceInfo.tokenPerPoint *
_settledPoints;
ITokenManager tokenManager = tadleFactory.getTokenManager();
if (settledPointTokenAmount > 0) {
tokenManager.tillIn(
>>> _msgSender(),
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
// ...
}
// ...
}

only the owner of the offer can call the settleAskTaker function and this function forces the owner of the offer to transfer points to the capitalPool in the amount of settledPointTokenAmount. this is unintended cause bid-makers provide and inject erc20 tokens to capitalPool to receive points.
if they have any points before the trade process, they will lose them. if they do not, it will get reverted.

This test demonstrates the scenario by adding it to the PreMarkets.t.sol:

function test_bidMakersMustPayPointsToBuyPoints() public {
address _offer1 = GenerateAddress.generateOfferAddress(0);
address _stock2 = GenerateAddress.generateStockAddress(1);
uint256 userPointsBalBef = mockPointToken.balanceOf(user);
vm.prank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
10000,
300,
OfferType.Bid,
OfferSettleType.Protected
));
vm.prank(user2);
preMarktes.createTaker(_offer1, 1000);
vm.startPrank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
vm.stopPrank();
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
deliveryPlace.settleAskTaker(_stock2, 1000);
uint256 userPointsBalAf = mockPointToken.balanceOf(user);
vm.stopPrank();
assertTrue(userPointsBalAf < userPointsBalBef);
}

simply run:

forge test --mt test_bidMakersMustPayPointsToBuyPoints -vv

Impact

bid-makers Must pay points to buy points, which leads to loss of funds or revert if they don't have any

Tools Used

manual review, unit tests

Recommendations

remove the whole tillIn external call from the `DeliveryPlace::settleAskTaker because the bid-maker shouldn't inject points to the capitalPool

Updates

Lead Judging Commences

0xnevi Lead Judge
about 1 year ago
0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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