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

`settleAskMaker()` would never work when `marketPlaceInfo.tokenAddress` is WETH

bauchibred

Summary

settleAskMaker() never works when marketPlaceInfo.tokenAddress is WETH cause of the wrong implementation in tillIn()

Vulnerability Details

When settling, this is queried

function settleAskMaker(address _offer, uint256 _settledPoints) external {
..snip
ITokenManager tokenManager = tadleFactory.getTokenManager();
if (settledPointTokenAmount > 0) {
tokenManager.tillIn(
_msgSender(),
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
}
..snip
}

However when the token is WETH which is supported, or otherwise wrapped ether, the execution goes under this block:

function tillIn(
address _accountAddress,
address _tokenAddress,
uint256 _amount,
bool _isPointToken
)
external
payable
onlyRelatedContracts(tadleFactory, _msgSender())
onlyInTokenWhiteList(_isPointToken, _tokenAddress)
{
/// ..snip
if (_tokenAddress == wrappedNativeToken) {
//@audit
if (msg.value < _amount) {
revert Errors.NotEnoughMsgValue(msg.value, _amount);
}
IWrappedNativeToken(wrappedNativeToken).deposit{value: _amount}();
_safe_transfer(wrappedNativeToken, capitalPoolAddr, _amount);
} else {
/// ..snip
}
emit TillIn(_accountAddress, _tokenAddress, _amount, _isPointToken);
}

And this check would always revert the query, cause no msg.value was passed on when calling tillIn() from here.

Impact

Settling never works when marketPlaceInfo.tokenAddress is WETH.

NB: A similar bug case is applicable here too https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L377-L383

tokenManager.tillIn(
_msgSender(),
marketPlaceInfo.tokenAddress,
settledPointTokenAmount,
true
);
​

Tools Used

Manual review

Recommendations

Consider removing the msg.value check when the token is WETH.

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year 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.