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

The `DeliveryPlace.closeBidTaker` and `DeliveryPlace.settleAskTaker` functions adds `makerInfo.tokenAddress` tokens instead of `marketPlaceInfo.tokenAddress` tokens.

kupiasec

Github link

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

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

Summary

The DeliveryPlace.closeBidTaker and DeliveryPlace.settleAskTaker functions add makerInfo.tokenAddress tokens, and makerInfo.tokenAddress is not point token.
As a result, in the DeliveryPlace.closeBidTaker, stockInfo.authority does not receive point token.
And in the DeliveryPlace.settleAskTaker, offerInfo.authority does not receive point token.

Vulnerability Details

The DeliveryPlace.closeBidTaker function adds the pointTokenAmount tokens to stockInfo.authority, but added token is makerInfo.tokenAddress tokens, not point token from L198.

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

tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
L198: makerInfo.tokenAddress,
pointTokenAmount
);

The DeliveryPlace.settleAskTaker function adds the settledPointTokenAmount tokens to offerInfo.authority, but added token is makerInfo.tokenAddress tokens, not point token from L387.

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

tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
offerInfo.authority,
L387: makerInfo.tokenAddress,
settledPointTokenAmount
);

Impact

stockInfo.authority receives makerInfo.tokenAddress tokens instead of point tokens in the DeliveryPlace.closeBidTaker.
offerInfo.authority receives makerInfo.tokenAddress tokens instead of point tokens in the DeliveryPlace.settleAskTaker.

Tools Used

Manual Review

Recommendations

It is recommended to change the code as following:

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

tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
- makerInfo.tokenAddress,
+ marketPlaceInfo.tokenAddress,
pointTokenAmount
);

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

tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
offerInfo.authority,
- makerInfo.tokenAddress,
+ marketPlaceInfo.tokenAddress,
settledPointTokenAmount
);
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.