Tadle

Tadle
DeFi
30,000 USDC
View results
Submission Details
Severity: high
Valid

Incorrect updating of balance in deliveryplace.sol

Summary

The balance of a user is updated in the collateral token type instead of the point token type. This would lead to incorrect calculation of funds.

Vulnerability Details

In the closeBidTaker() function, the pointTokenAmountis added to the makerInfo.tokenAddress balance of the user.

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

tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
makerInfo.tokenAddress, // this is the token address of the collateral token
pointTokenAmount
);

This could lead to many issues, one of which is discussed below:

If the point token value is worth less than the collateral token (1000 point token = 1 USDC; usds is taken as an example), then the user will get more than they deserve.

Note this mistake is made in the settleAstTaker function also.

Impact

This leads to incorrect fund calculation(either more or less).

Tools Used

Manual Review

Recommendations

Update the code as follows:

tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
marketplaceInfo.tokenAddress, // correct point token address
pointTokenAmount
);
Updates

Lead Judging Commences

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