- 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
- 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.
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.
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.
- 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 :
- 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 :
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 :
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.