Erroneous liquidity data could be added to BalLiquidityProvider

[code423n4]

Lines of code

https://github.dev/code-423n4/2022-05-aura/blob/a8758161373bc9c9ad2aec363b511afa3ed0613f/contracts/BalLiquidityProvider.sol#L52

Vulnerability details

Impact

In the comment of the method provideLiquidity it said, but erroneous data could be added.

Provides liquidity on behalf of the dao, in a non-custodial manner.
Has protections in place to ensure that no erroneous liquidity data gets added.

Proof of Concept

The unique check of the assets is:

require(asset == address(startToken) || asset == address(pairToken), "!asset");

Therefore, the token in the first position is not checked to be the same as the token in the last position, if the tokens are repeated, all constraints will be bypassed.

Looking in the MockBalancerVault logic, it will be added as liquidity, only taking care about the balance of one token.

Source code:

• BalLiquidityProvider.sol#L52
• MockBalancerVault.sol#L46

Recommended Mitigation Steps

Check that _request.assets[0] != _request.assets[1]

[0xMaharishi]

This might cause the tx to fail but it wouldn't cause any assets to get locked or for any funds to go missing so would be a 1 severity at most

[dmvt]

Agree with sponsor. This is a QA issue.

[dmvt]

Grouping this with the warden’s QA report, #83