sujithsomraaj / lifi-emergency-pause-facet-v1.2.0

1 day review - 1 / 09
0 stars 0 forks source link

Potential accidental removal of `DiamondCutFacet` during unpause #13

Open sujithsomraaj opened 1 week ago

sujithsomraaj commented 1 week ago

Context: EmergencyPauseFacet.sol#L140

Description: The unpauseDiamond function allows for a blacklist of facets that should not be reactivated. However, there's no safeguard to prevent the DiamondCutFacet from being included in this blacklist, which could lead to the accidental permanent disabling of upgrade functionality.

Recommendation: Implement a check to ensure that the DiamondCutFacet is never included in the blacklist.

function unpauseDiamond(address[] calldata _blacklist) external {
+  bytes4[] memory selectors;
    for (uint256 i; i < _blacklist.length; ) {
+  selectors = LibDiamondLoupe.facetFunctionSelectors(_blacklist[i]);
+  if(selectors[0] == DiamondCutFacet.diamondCut.selector) continue;
      // re-add facet and its selectors to diamond
      LibDiamond.removeFunctions(
         address(0),
+        selectors
       );
}

LI.FI: Fixed in 7709442ae76b0209a93c732c412fcb444216c618

Researcher: Confirmed.

0xDEnYO commented 1 week ago

well-spotted. Implemented as suggested (also updated the wrong comment inside the loop). Fix: https://github.com/lifinance/contracts/commit/7709442ae76b0209a93c732c412fcb444216c618