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

6 stars 4 forks source link

LiFi data disjoint from swapData and other data used in transaction logic #137

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Interfaces/ILiFi.sol#L7 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L35 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/AnyswapFacet.sol#L74-L78 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/CBridgeFacet.sol#L57 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/CBridgeFacet.sol#L92-L96 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/GenericSwapFacet.sol#L22 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/HopFacet.sol#L61 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/HopFacet.sol#L95-L99 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/NXTPFacet.sol#L46 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/NXTPFacet.sol#L85-L89 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/NXTPFacet.sol#L124-L129 https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/NXTPFacet.sol#L150-L155

Vulnerability details

Impact

In document: https://github.com/code-423n4/2022-03-lifinance/blob/main/docs/HopFacet.md#lifi-data

## LiFi Data

Some methods accept a `LiFiData _lifiData` parameter.

This parameter is strictly for analytics purposes. It's used to emit events that we can later track and index in our subgraphs and provide data on how our contracts are being used. `LiFiData` and the events we can emit can be found here.

Presumably, the SDK uses LiFi data to keep track of on-chain activities, however, there are no guarantees that LiFi data must match other data actually used in transactions. Thus one can simply file incorrect LiFi data to trick analytics into believing some other transactions happened instead.

Proof of Concept

  1. Craft random or obfuscated LiFiData struct
  2. Call facets like AnyswapFacet, CBridgeFacet, GenericSwapFacet, HopFacet, NXTPFacet that use LiFiData
  3. Contract will emit LiFi event but data in the event is disjoint from actual swap data

Tools Used

hardhat

Recommended Mitigation Steps

Use swapData or variables which are actually used in transactions for LiFi events.

H3xept commented 2 years ago

Duplicate of #62

gzeoneth commented 2 years ago

Agree with sponsor this does not pose a functional risk. Changing to Low/QA.

gzeoneth commented 2 years ago

Consider with warden's QA Report #139