Platform: Code4rena
Start Date: 30/10/2021
Pot Size: $35,000 ETH
Total HM: 2
Participants: 16
Period: 3 days
Judge: alcueca
Total Solo HM: 1
Id: 48
League: ETH
Rank: 5/16
Findings: 2
Award: $2,993.71
🌟 Selected for report: 1
🚀 Solo Findings: 0
🌟 Selected for report: daejunpark
Also found by: pauliax
897.4913 USDC - $897.49
pauliax
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
Remove 'payable' from the definition of the swap function.
#0 - tommyz7
2021-11-04T17:08:10Z
Solidity throws an error if swap function is not payable.
#1 - tommyz7
2021-11-04T18:11:12Z
I might have been in old version of solidity tho, will have to double check
#2 - alcueca
2021-11-09T05:43:13Z
Duplicate of #25
33.9293 USDC - $33.93
pauliax
functions registerSwapModuleBatch and unregisterSwapModuleBatch do not really need an 'onlyAdmin' modifier as this will be checked in registerSwapModule/unregisterSwapModule anyway.
Consider removing this modifier from the declaration to save some gas.
33.9293 USDC - $33.93
pauliax
Using the 'unchecked' keyword to avoid redundant arithmetic underflow/overflow checks to save gas when an underflow/overflow cannot happen. Unchecked can be applied here:
if (currentAllowance < amount) { // ... token.safeIncreaseAllowance(spender, amount - currentAllowance); }
Consider using unchecked maths where gas costs are important and overflow/underflow are not possible.
33.9293 USDC - $33.93
pauliax
contract Slingshot is ConcatStrings for no reason as it does not use any of the functions provided by this contract.
To save some gas consider removing this useless inheritance.
🌟 Selected for report: pauliax
1994.4252 USDC - $1,994.43
pauliax
Slingshot contract does not need a 'receive' function as it is not supposed to receive ETH directly. Executioner has this function too and it needs to receive ETH from the WETH contract. Because it expects only WETH to send the native asset directly, it should check that the msg.sender is actually WETH contract.
receive() external payable { require(msg.sender == wrappedNativeToken, "..."); }