code-423n4 / 2021-10-tally-findings

0 stars 0 forks source link

msg.value should be 0 when token is not ETH #53

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

pauliax

Vulnerability details

Impact

If the token to be sold is not ETH, it should check that msg.value is 0 to prevent accidentally sent ETH.

Recommended Mitigation Steps

Proposed improvement: require( (!signifiesETHOrZero(zrxSellTokenAddress) && msg.value = 0) || msg.value > 0, "Swap::swapByQuote: Unwrapped ETH must be swapped via msg.value" );

Shadowfiend commented 2 years ago

The 0x API occasionally requires protocol fees in ETH (see https://0x.org/docs/guides/swap-tokens-with-0x-api#paying-protocol-fees ).

0xean commented 2 years ago

closing as 0x spec requires this for fees.