LI.FI contest - nedodn's results

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Libraries/LibSwap.sol#L42 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/DexManagerFacet.sol#L17 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/Swapper.sol#L15

Vulnerability details

Impact

There is a common vulnerability with aggregator/bridge contracts where passing in arbitrary calldata can do unwanted actions such as steal tokens that were approved to that contract. While there is a whitelist system set up, there is no stopping a malicious or comprised owner account from adding contracts to that list and executing these calls themselves. While the owners may be trustworthy, when the risks are total loss of funds for a user due to an approval (commonly used in the space and left) risk of external compromises should be taken seriously (see recent ronin/axs hack)

Proof of Concept

1. Owner adds token contracts and transferFrom calldata to whitelist
2. Owner inserts transferFrom data into _swapData
3. Owner executes a swap that steals tokens that have been approved by users.

Tools Used

Line by line analysis

Recommended Mitigation Steps

While I will admit these types of issues are tough to solve without some degree of trust, one can reduce the risk by adding in a timelock to the whitelist so that owners have a way to monitor if a malicious owner is attempting to steal.