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

0 stars 0 forks source link

payable swap #93

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

pauliax

Vulnerability details

Impact

The swap() function in the modules is payable although it does not use the msg.value and operates on WETH. Slingshot contract handles all the conversions ETH <-> WETH so modules should only expect WETH. Declaring a function as payable means it can also receive ETH. However, there is no function to later extract ETH which is accidentally sent to this function, leaving ETH stuck there forever.

This issue was also submitted in the previous Slingshot contest and was assigned a severity of low: https://ipfs.io/ipfs/bafybeicjla2h26q3wz4s344bsrtvhkxr3ypm44owvrzyorb2t6tcptlmem/C4%20Slingshot%20report.pdf

Recommended Mitigation Steps

Remove 'payable' from the definition of the swap function.

tommyz7 commented 2 years ago

Solidity throws an error if swap function is not payable.

tommyz7 commented 2 years ago

I might have been in old version of solidity tho, will have to double check

alcueca commented 2 years ago

Duplicate of #25