sujithsomraaj / lifi-stargate-v2-audit

3 Day Review [10 Jun 2024 - 13 Jun 2024]
0 stars 0 forks source link

Inconsistent `msg.value` validations in `startBridgeTokensViaStargate` function #12

Open sujithsomraaj opened 5 months ago

sujithsomraaj commented 5 months ago

Context: StargateFacetV2.sol#L48

Description: The function startBridgeTokensViaStargate bridges native token and ERC20 tokens via stargate bridge. This issue primarily impacts users while bridging native tokens via Stargate due to inconsistent msg.value validations.

The function before calling Stargate validates the msg.value if it's more significant than the _bridgeData.minAmount; however, the exact msg.value should be greater than _bridgeData.minAmount and stargateData.fee.nativeValue as Stargate expects a native fee payment.

LibAsset.depositAsset(
  _bridgeData.sendingAssetId,
  _bridgeData.minAmount
);

As a result, the user could either avoid paying stargate fees by utilizing the native tokens present in the diamond contract`(very rare case) (or) will get an unhandled error (out of funds).

The issue is also present for the ERC20 token bridging using the same function, as the msg.value is not validated to ensure the user pays appropriate Stargate fees.

Recommendation: Consider adding validations to ensure the user pays Stargate fees.

LI.FI:

Researcher:

0xDEnYO commented 5 months ago

Hi @sujithsomraaj ,

these are good and valid points but in both cases the transactions would fail (I added test cases to confirm that) and we prioritize gas saving over extensive parameter checking.

I will leave this open for another team member to confirm but from my side we can close this without further action.

0xDEnYO commented 5 months ago

@ezynda3 if you agree, could you please close this issue?

ezynda3 commented 5 months ago

Skimming dust is a known issue which we accept and having transactions revert in the case of incorrect parameters is also acceptable. Closing.