code-423n4 / 2021-11-vader-findings

0 stars 0 forks source link

All user assets which are approved to VaderPoolV2 may be stolen #221

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

TomFrench

Vulnerability details

Impact

Total loss of funds which have been approved on VaderPoolV2

Proof of Concept

VaderPoolV2 allows minting of fungible LP tokens with the mintFungible function

https://github.com/code-423n4/2021-11-vader/blob/607d2b9e253d59c782e921bfc2951184d3f65825/contracts/dex-v2/pool/VaderPoolV2.sol#L284-L290

Crucially this function allows a user supplied value for from which specifies where the nativeAsset and foreignAsset should be pulled from. An attacker can then provide any address which has a token approval onto VaderPoolV2 and mint themselves LP tokens - stealing the underlying tokens.

Recommended Mitigation Steps

Remove from argument and use msg.sender instead.

SamSteinGG commented 2 years ago

pool is not meant to be interacted with

alcueca commented 2 years ago

And how are you going to ensure that the pool is not interacted with, @SamSteinGG?

SamSteinGG commented 2 years ago

@alcueca Upon second consideration, the functions relating to the minting of synths and wrapped tokens should have had the onlyRouter modifier and thus are indeed vulnerable. Issue accepted.