Github link
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L303
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L403
Summary
In settleAskMaker and settleAskTaker function, collaterals are refunded to msg.sender and msg.sender may be owner.
But, collaterals should be refunded to authority instead of owner.
Vulnerability Details
In settleAskMaker and settleAskTaker function, collaterals are refunded to msg.sender and msg.sender may be owner.
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L304
tokenManager.addTokenBalance(
TokenBalanceType.SalesRevenue,
_msgSender(),
makerInfo.tokenAddress,
makerRefundAmount
);
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L404
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash,
_msgSender(),
makerInfo.tokenAddress,
collateralFee
);
Impact
This causes the user's loss of funds.
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/PreMarkets.sol#L304
tokenManager.addTokenBalance(
TokenBalanceType.SalesRevenue,
- _msgSender(),
+ offerInfo.authority,
makerInfo.tokenAddress,
makerRefundAmount
);
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L404
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash,
- _msgSender(),
+ stockInfo.authority,
makerInfo.tokenAddress,
collateralFee
);