search

code-423n4 / 2022-03-lifinance-findings

6 stars 4 forks source link

Compromised governance can potentially steal user fund #80

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L17-L26 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/GenericSwapFacet.sol#L22-L43

Vulnerability details

Impact

GenericSwapFacet.swapTokensGeneric() can run arbitrary calls on any whitelisted addresses. A malicious/compromised governance can whitelist token contracts and steal funds from users that have given allowance to LiFi contract.

Proof of Concept

  1. Alice approves infinite USDC allowance to LiFi.
  2. Governance calls addDex(), whitelisting USDC contract.
  3. Governance calls swapTokensGeneric() with USDC as the callTo and transferFrom as the callData, stealing Alice's fund.

Recommended Mitigation Steps

Recommend using timelock when managing dexWhitelist.

H3xept commented 2 years ago

Duplicate of #65