LI.FI contest - PranavG's results

- Calls to whitelisted EAO can give token approval to EAO (non-critical) If admin whitelists a non-contract address, low level calls in the LibSwap.sol library will always return true, and the whole transaction will go through without any revert. This can have two issues : - 'msg.value' amount of ether will be sent to that address, which is not the required behaviour - approvalTo can be that EOA address. This means in future if there are any locked funds in this contract then, the EOA can easily transferFrom it. Reference : https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L42 Mitigation : Use OZ's implementation for low level calls with value which checks if the address is a contract or not. Otherwise, while whitelisting check if the address is a contract or not.

- Contract can lock user funds, if more than one value passed in swapData (Low) #33 LibSwap.sol checks if the contract balance is less than transfer amount, then it transfers from the msg.sender. Here it will transferFrom the whole amount, not the difference between the two. This will have an issue : - User wants to do swap from A -> C, and is getting a very good price on a path : A -> B -> C. So the swapData will have 2 elements having data for the two swaps on an exchange. First swap will send the tokens back to Lifi contract and then use them for second swap i.e. B -> C. Now if B received after first swap was less than amount to be used for B -> C, then transferFrom msg.sender to contract will happen which will transfer the whole amount required for B -> C swap. (given the msg.sender did approve for B token previously). This will lock the tokens, received from A -> B swap, in the contract. - The locked tokens can be used by any user to swap without even transferring any token, given the transferAmount is less than the contract balance. Reference : https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L33 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L34

Mitigation : transferFrom the difference of the contract balance and required amount. This will help in compatibility with different exchanges and different types of swapData as well.

- Do not send msg.value for all the low level calls for swap (Low) #43 LibSwap.sol The swap function is called for every element in the swapData array. But the swap function, when making a low level call to the DEX, will always send the 'msg.value' amount of ether for the swap. This will have issues :

• You cannot send msg.value for every swap, as user sent that amount only once and contract won't hold any ETH balance. This means swaps related to Eth will not support multiple elements in the swapData array.
• Even if the contract holds the ETH balance, there can be a swapData array where functions called will be payable and non-payable both, and sending msg.value to non-payable functions will revert. This means you cannot merge payable and non payable swaps in one transaction.

Reference : https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L42

Mitigation : Only send ETH for the swap where the fromToken is native asset. Do not send the whole msg.value, instead send the required fromToken only and return the excess msg.value to the msg.sender.