Tadle

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

SettleAskTaker and SettleAskMaker will revert when marketplaceInfo.tokenAddress = wETH and settlePointTokenAmount > 0

Summary

Due to missing "payable" modifier, user won't be able to deposit Eth to the SettleAskTaker and SettleAskMaker, thereby causing the functions to revert when marketplaceInfo.tokenAddress == wETH and settlePointTokenAmount > 0

Vulnerability Details

In the SettleAskMaker and SettleAskTaker function when settlePointTokenAmount > 0

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

Users need to deposit settlePointTokenAmount in other to get their pointToken balance

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

But this process will always revert if marketplaceInfo.tokenAddress == wETH because in order to deposit wETH to the protocol user will have to send eth directly to the protocol, as seen in the tillIn function

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L79-L90

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L56-L103

This process will always revert resulting in dos of the SettleAskMaker and SettleAskTaker function due to missing payable modifier

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

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

Impact

Dos in SettleAskTaker and SettleAskMaker when marketplaceInfo.tokenAddress is wETH and settlePointTokenAmount > 0

Tools Used

Manual review

Recommendations

Add payable modifier to the SettleAskTaker and SettleAskMaker function and also option to send Eth via {value: msg.value} should be added to the point where tillIn function called inside both functions

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

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

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-TokenManager-tillin-wrapper-inconsistent

Valid medium severity, given it is noted in contest READ.ME that any standard ERC20 tokens should be supported. Although arguably could be low severity, given users can simply unwrap WETH to native ETH and perform the deposits via `tillIn()`, I will leave open for discussions, but taking READ.ME as the source of truth, I believe medium severity is appropriate, given it is explicitly noted that this token should be compatible#9##. The fix would be to utilize a zero address or equivalent to represent native ETH when wrapping to WETH. > Tokens: - ETH - WETH - ERC20 (any token that follows the ERC20 standard)

Support

FAQs

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