sujithsomraaj / lifi-stargate-v2-audit

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

Validate if `stargateData.assetId` and `_bridgeData.sendingAssetId` matches #15

Open sujithsomraaj opened 5 months ago

sujithsomraaj commented 5 months ago

Context: StargateFacetV2.sol#L105

Description: The _startBridge internal function initiates a bridging process using Stargate bridge. The function receives two types of assets for sending from _bridgeData.sendingAssetId and _stargateData.assetId, which could be unrelated.

Any discrepancies in both values could lead to unexpected behavior. Ideally, the diamond contract holds no funds and hence the function call should revert, but an additional check could prevent future issues.

Recommendation: It's more logical to either have an internal mapping / read the token() from the router address to ensure the right asset is passed in both parameters.

// get the address of the Stargate pool/router contract
address routerAddress = _getRouterAddress(_stargateData.assetId);
+ require(_bridgeData.sendingAssetId == IPool(routerAddress).token());

LI.FI:

Researcher:

0xDEnYO commented 5 months ago

I see your point but I feel that this is an additional check which would not provide a huge value. If bridgeData.sendingAssetId will not match with the tokenAddress of the pool then the whole tx will fail (e.g. the deposited token would not match with the token to be bridged etc.).

Can you construct a case where this is not true, @sujithsomraaj ?

sujithsomraaj commented 5 months ago

As I mentioned in the report, it'll revert, so if you think it's a gas overhead, then fine, you could avoid adding this.

0xDEnYO commented 5 months ago

OK. We will close this issue then as we prefer to save the gas. Thanks for pointing it out though.