search

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

0 stars 0 forks source link

Extra transfers when burning LP tokens #250

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

TomFrench

Vulnerability details

Impact

Gas costs

Proof of Concept

When burning ERC20 LP tokens VaderPoolV2 first transfers them from the user to itself and then burns them.

https://github.com/code-423n4/2021-11-vader/blob/429970427b4dc65e37808d7116b9de27e395ce0c/contracts/dex-v2/pool/VaderPoolV2.sol#L365-L366

As LPToken trusts VaderPoolV2 a cheaper way to do this would be to burn them directly from the user's balance. This avoids the cost of approving VaderPoolV2 to take the tokens and transferring them

Recommended Mitigation Steps

Allow VaderPoolV2 to directly burn LP tokens from users accounts and skip the transfer.

SamSteinGG commented 2 years ago

We understand the gas optimization here but we are going to keep it as it is for standartization.