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

`settleAskTaker()` transfers tokens from `offerInfo.authority` and allocated to the same address, instead of `stockInfo.authority`

100HP

Vulnerability Details

A trader places a buy offer via the createOffer() function, on which ask orders are placed by takers through a call to createTaker(). Multiple ask orders can be placed on the offer, and it is up to the maker to decide which _stockId and for how many points those ask orders need to be settle.

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

During a settleAskTaker(), an equivalent cash proportional to the _settlePoints is transferred from the offer maker to the tokenManager contract,

uint256 settledPointTokenAmount = marketPlaceInfo.tokenPerPoint *
_settledPoints;

instead of adding that balance to the taker balance, its mistakenly adding to the offerInfo.authority which is same as maker or caller.

tokenManager.tillIn( // @audit transfer from maker
_msgSender(),
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
​
tokenManager.addTokenBalance( // @audit allocating to maker, instead of the stock owner
TokenBalanceType.PointToken,
offerInfo.authority,
makerInfo.tokenAddress,
settledPointTokenAmount
);

This cause taker order to be mark FINISHED, without having any PointToken to be added in their accountAddress, result a direct fund loss.

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

Impact

Maker settle taker orders, but those funds never get added to their accountAddress.

Tools Used

Manual review

Recommendations

Modify following line,

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

tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
- offerInfo.authority,
+ stockInfo.authority,
makerInfo.tokenAddress,
settledPointTokenAmount
);
Updates

Lead Judging Commences

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

finding-DeliveryPlace-settleAskTaker-closeBidTaker-wrong-makerinfo-token-address-addToken-balance

Valid high severity, In `settleAskTaker/closeBidTaker`, by assigning collateral token to user balance instead of point token, if collateral token is worth more than point, this can cause stealing of other users collateral tokens within the CapitalPool contract, If the opposite occurs, user loses funds based on the points they are supposed to receive

Support

FAQs

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