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

The collateral amount will not calculated to the maker if settled Amount is equal to usedPoints.

pavankv

Summary

The collateral fee amount will not calculated to the maker if he was settled Amount is equal to usedPoints when the settling as maker.

Vulnerability Details

In settleAskMaker() function have a check that settlePoints should not more that offer used points then settle points can be equal to used Points.

if (_settledPoints > offerInfo.usedPoints) {
revert InvalidPoints();
}

https://github.com/pavankv241/Tadle-ICP/blob/main/src/core/DeliveryPlace.sol#L230C1-L232C10

Another check which we have to look into that if settle points equal to the used points then makerRefund Amount will be calculated as sales revenue to the maker.

if (_settledPoints == offerInfo.usedPoints) {
if (offerInfo.offerStatus == OfferStatus.Virgin) {

https://github.com/pavankv241/Tadle-ICP/blob/main/src/core/DeliveryPlace.sol#L276C8-L306C15

But when the maker try to close the offer after several trade and decided to settle as full used points then no collateral fee amount will calculated due to the below check

uint256 collateralFee;
if (offerInfo.usedPoints > offerInfo.settledPoints) {

https://github.com/pavankv241/Tadle-ICP/blob/main/src/core/DeliveryPlace.sol#L152C1-L153C1

The above check allows only if the settle amount less than used amount here but maker intended to completely settle the offer.

The another is issue here is collateral fee amount will be same if settle amount lesser than the user points which means

Ex :-

If the used points are 500 and any settle amount less than 500 will be 6000000000000000, which will uneconomically incentivize the maker, then the maker can settle each single amount. Every time, 6e15 will be added to the maker's account, which is more than his traded volume in his offer.

Impact

The user Collateral fee amount will not be added if the Maker try to settled as used points of offer which will be loss to maker.

Proof of Concept

Add this function test file run below command :-

forge test --match-test test_CheckCollateralFee -vvv

function test_CheckCollateralFee() public {
​
vm.startPrank(user);
​
preMarktes.createOffer{value: 0.012 * 1e18}(
CreateOfferParams(
marketPlace,
address(weth9),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker{value: 0.005175 * 1e18}(offerAddr, 500);
​
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer(stock1Addr, 0.006 * 1e18, 12000); // Selling some points
​
address offer1Addr = GenerateAddress.generateOfferAddress(1);
preMarktes.closeOffer(stock1Addr, offer1Addr);
preMarktes.relistOffer(stock1Addr, offer1Addr);
​
vm.stopPrank();
​
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
​
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
deliveryPlace.settleAskMaker(offerAddr, 100); // Settled Points 500
deliveryPlace.closeBidTaker(stock1Addr);
​
StockInfo memory stockInfo = preMarktes.getStockInfo(stock1Addr);
(
OfferInfo memory offerInfo,
MakerInfo memory makerInfo,
,
) = deliveryPlace.getOfferInfo2(stockInfo.preOffer);
​
console.log("The used points of offer is" , offerInfo.usedPoints);
​
uint256 RemainIngAmount = tokenManager.getTokenBalance(TokenBalanceType.RemainingCash, address(user), makerInfo.tokenAddress);
console.log("The Collateral Fee which has been added",RemainIngAmount);
vm.stopPrank();
}

Please check the @audit comment in above POC

Output

// This output when the maker try to settle lower than usedPoints
Ran 1 test for test/PreMarkets.t.sol:PreMarketsTest
[PASS] test_CheckCollateralFee() (gas: 1413635)
Logs:
The used points of offer is 500
The Collateral Fee which has been minted 6000000000000000
​
// This output when the maker try to settle equal to used Points
// try change the settle amount in test case as 499, 498
Ran 1 test for test/PreMarkets.t.sol:PreMarketsTest
[PASS] test_CheckCollateralFee() (gas: 1400013)
Logs:
The used points of offer is 500
The Collateral Fee which has been minted 0

Tools Used

Foundry , Manual View

Recommendations

Check changed to add collateral fee even the offer traded sometimes at the end maker try to settle as equal to used points of offer which will be incentive maker as economically.

if (offerInfo.usedPoints >= offerInfo.settledPoints) {
Updates

Lead Judging Commences

0xnevi Lead Judge 12 months 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

0xnevi Lead Judge 11 months 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.