code-423n4 / 2022-03-lifinance-findings

6 stars 4 forks source link

Incorrect implementation of the `batchRemoveDex` function in `DexManagerFacet` #208

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/main/src/Facets/DexManagerFacet.sol#L73

Vulnerability details

Impact

The batchRemoveDex function does not work as expected. It should remove all the given DEX addresses from the dexWhitelist. However, it only removes the first successfully found DEX address and then stops removing the rest. The functionality is broken, and therefore the admin can only remove one DEX address at a time.

Proof of Concept

There is a return statement at line 73, so after the first DEX address is found in the array and is removed, the function directly returns.

DexManagerFacet.sol#L73

Recommended Mitigation Steps

Consider removing line 73 (the return in the function).

H3xept commented 2 years ago

Duplicate of #34

gzeoneth commented 2 years ago

Changing to Med Risk to align with #34