VaderReserve does not support paying IL protection out to more than one address, resulting in locked funds

[code423n4]

Handle

TomFrench

Vulnerability details

Impact

All liquidity deployed to one of VaderPool or VaderPoolV2 will be locked permanently.

Proof of Concept

Both VaderRouter and VaderRouterV2 make calls to VaderReserve in order to pay out IL protection.

https://github.com/code-423n4/2021-11-vader/blob/3a43059e33d549f03b021d6b417b7eeba66cf62e/contracts/dex/router/VaderRouter.sol#L206

https://github.com/code-423n4/2021-11-vader/blob/3a43059e33d549f03b021d6b417b7eeba66cf62e/contracts/dex-v2/router/VaderRouterV2.sol#L227

However VaderReserve only allows a single router to claim IL protection on behalf of users.

https://github.com/code-423n4/2021-11-vader/blob/3a43059e33d549f03b021d6b417b7eeba66cf62e/contracts/reserve/VaderReserve.sol#L80-L83

It's unlikely that the intent is to deploy multiple reserves so there's no way for both VaderRouter and VaderRouterV2 to pay out IL protection simultaneously.

This is a high severity issue as any LPs which are using the router which is not listed on VaderReserve will be unable to remove liquidity as the call to the reserve will revert. Vader governance is unable to update the allowed router on VaderReserve so all liquidity on either VaderPool or VaderPoolV2 will be locked permanently.

Recommended Mitigation Steps

Options:

1. Allow the reserve to whitelist multiple addresses to claim funds
2. Allow the call to the reserve to fail without reverting the entire transaction (probably want to make this optional for LPs)

[SamSteinGG]

As the code indicates, only one of the two versioned instances of the AMM will be deployed and active at any given time rendering this exhibit incorrect.

[alcueca]

Sorry @SamSteinGG, where does the code indicate that?

[SamSteinGG]

Correction, this was clarified during the audit in the discord channel.