Beanstalk Notion
Beanstalk Notion
/
🪲
Bug Reports
/
BIC Notes
/
📄
Report #30181
📄

Report #30181

Report Date
April 18, 2024
Status
Closed
Payout

Missing address length check leads to smart contract failed to deliver promise result

‣
Report Info

Report ID#30181

Report type

Smart Contract

Has PoC?

Yes

Target

https://etherscan.io/address/0xDEb0f00071497a5cc9b4A6B96068277e57A82Ae2

Impacts

  • Contract fails to deliver promised returns, but doesn't lose value

Description

Hello team,

I would like to report a bug in smart contract in which developer miss to implement an address.length should not be Zero which will leads smart contract failed to delivered promise result but doesn’t loose value.

Link = https://etherscan.io/address/0xDEb0f00071497a5cc9b4A6B96068277e57A82Ae2?utm_source=immunefi#code

At File 22 :

Line 182 ,

function removeFunction(DiamondStorage storage ds, address _facetAddress, bytes4 _selector) internal {

require(_facetAddress != address(0), "LibDiamondCut: Can't remove function that doesn't exist");

// an immutable function is a function defined directly in a diamond

require(_facetAddress != address(this), "LibDiamondCut: Can't remove immutable function");

// replace selector with last selector, then delete last selector

uint256 selectorPosition = ds.selectorToFacetAndPosition[_selector].functionSelectorPosition;

uint256 lastSelectorPosition = ds.facetFunctionSelectors[_facetAddress].functionSelectors.length - 1;

// if not the same then replace _selector with lastSelector

if (selectorPosition != lastSelectorPosition) {

bytes4 lastSelector = ds.facetFunctionSelectors[_facetAddress].functionSelectors[lastSelectorPosition];

ds.facetFunctionSelectors[_facetAddress].functionSelectors[selectorPosition] = lastSelector;

ds.selectorToFacetAndPosition[lastSelector].functionSelectorPosition = uint96(selectorPosition);

}

// delete the last selector

ds.facetFunctionSelectors[_facetAddress].functionSelectors.pop();

delete ds.selectorToFacetAndPosition[_selector];

// if no more selectors for facet address then delete the facet address

if (lastSelectorPosition == 0) {

// replace facet address with last facet address and delete last facet address

uint256 lastFacetAddressPosition = ds.facetAddresses.length - 1;

uint256 facetAddressPosition = ds.facetFunctionSelectors[_facetAddress].facetAddressPosition;

if (facetAddressPosition != lastFacetAddressPosition) {

address lastFacetAddress = ds.facetAddresses[lastFacetAddressPosition];

ds.facetAddresses[facetAddressPosition] = lastFacetAddress;

ds.facetFunctionSelectors[lastFacetAddress].facetAddressPosition = facetAddressPosition;

}

ds.facetAddresses.pop();

delete ds.facetFunctionSelectors[_facetAddress].facetAddressPosition;

}

}

Here In Code , There’s a missing check for length :

require(ds.facetFunctionSelectors[_facetAddress].functionSelectors.length  >0 );

require ( ds.facetAddresses.length > 0 );

Due to this missing check an Arithmetic Underflow issue occur since solidity version used here is 7.6.0 which is still vulnerable to overflow and underflow issue .

Vulnerability Details :

In Link = https://etherscan.io/address/0xDEb0f00071497a5cc9b4A6B96068277e57A82Ae2?utm_source=immunefi#code

At File 22 :

Line 182 ,

function removeFunction(DiamondStorage storage ds, address _facetAddress, bytes4 _selector) internal {

require(_facetAddress != address(0), "LibDiamondCut: Can't remove function that doesn't exist");

// an immutable function is a function defined directly in a diamond

require(_facetAddress != address(this), "LibDiamondCut: Can't remove immutable function");

// replace selector with last selector, then delete last selector

uint256 selectorPosition = ds.selectorToFacetAndPosition[_selector].functionSelectorPosition;

uint256 lastSelectorPosition = ds.facetFunctionSelectors[_facetAddress].functionSelectors.length - 1;

// if not the same then replace _selector with lastSelector

if (selectorPosition != lastSelectorPosition) {

bytes4 lastSelector = ds.facetFunctionSelectors[_facetAddress].functionSelectors[lastSelectorPosition];

ds.facetFunctionSelectors[_facetAddress].functionSelectors[selectorPosition] = lastSelector;

ds.selectorToFacetAndPosition[lastSelector].functionSelectorPosition = uint96(selectorPosition);

}

// delete the last selector

ds.facetFunctionSelectors[_facetAddress].functionSelectors.pop();

delete ds.selectorToFacetAndPosition[_selector];

// if no more selectors for facet address then delete the facet address

if (lastSelectorPosition == 0) {

// replace facet address with last facet address and delete last facet address

uint256 lastFacetAddressPosition = ds.facetAddresses.length - 1;

uint256 facetAddressPosition = ds.facetFunctionSelectors[_facetAddress].facetAddressPosition;

if (facetAddressPosition != lastFacetAddressPosition) {

address lastFacetAddress = ds.facetAddresses[lastFacetAddressPosition];

ds.facetAddresses[facetAddressPosition] = lastFacetAddress;

ds.facetFunctionSelectors[lastFacetAddress].facetAddressPosition = facetAddressPosition;

}

ds.facetAddresses.pop();

delete ds.facetFunctionSelectors[_facetAddress].facetAddressPosition;

}

}

Here In Code , There’s a missing check for length :

require(ds.facetFunctionSelectors[_facetAddress].functionSelectors.length  >0 );

require ( ds.facetAddresses.length > 0 );

Due to this missing check an Arithmetic Underflow issue occur since solidity version used here is 7.6.0 which is still vulnerable to overflow and underflow issue .

Impact :

  1. Smart contract fails to Delivered promise result but Doesn’t Loose value : in this case due to missing check for address

    2. 0 - 1 = Max Uint256

removeFunction() function contributes to the flexibility, efficiency, and long-term maintainability of Diamond contracts by enabling the removal of obsolete or unused functions from facets while ensuring the contract's structural integrity.

Recommendation :

I recommend Developer to introduce a require check in smart contract  removeFunction ().

require(ds.facetFunctionSelectors[_facetAddress].functionSelectors.length  >0 );

require ( ds.facetAddresses.length > 0 );

Thank You

Proof of concept

Step 1 : Go to Link = https://etherscan.io/address/0xDEb0f00071497a5cc9b4A6B96068277e57A82Ae2?utm_source=immunefi#code

Step 2 : At File 22 :

Line 182 ,

Step 3 : Here In Code , There’s a missing check for length :

require(ds.facetFunctionSelectors[_facetAddress].functionSelectors.length  >0 );

require ( ds.facetAddresses.length > 0 );

Due to this missing check an Arithmetic Underflow issue occur since solidity version used here is 7.6.0 which is still vulnerable to overflow and underflow issue .

Immunefi Response

Thank you for submitting your vulnerability report to the Beanstalk bug bounty program. We appreciate your efforts and taking the time to report vulnerabilities to us. We have reviewed your submission, but unfortunately, we are closing the report for the following reasons:
  • The submission contains the output of an automated scanner without demonstrating that it is a valid issue.
  • The submission lacks the required information regarding the vulnerability's impact on the reported asset.

As per the bug bounty program's policy, we require all submissions to be accompanied by a Proof of Concept (PoC) that demonstrates the vulnerability's existence and impact. Since the submission doesn't provide any proof of the vulnerability's existence, we have decided to close it.

Please note that the project will receive a report of the closed submission and may choose to re-open it, but they are not obligated to do so.

        function removeFunction(DiamondStorage storage ds, address _facetAddress, bytes4 _selector) internal {

    require(_facetAddress != address(0), "LibDiamondCut: Can't remove function that doesn't exist");

    // an immutable function is a function defined directly in a diamond

    require(_facetAddress != address(this), "LibDiamondCut: Can't remove immutable function");

    // replace selector with last selector, then delete last selector

    uint256 selectorPosition = ds.selectorToFacetAndPosition[_selector].functionSelectorPosition;

    uint256 lastSelectorPosition = ds.facetFunctionSelectors[_facetAddress].functionSelectors.length - 1;

    // if not the same then replace _selector with lastSelector

    if (selectorPosition != lastSelectorPosition) {

        bytes4 lastSelector = ds.facetFunctionSelectors[_facetAddress].functionSelectors[lastSelectorPosition];

        ds.facetFunctionSelectors[_facetAddress].functionSelectors[selectorPosition] = lastSelector;

        ds.selectorToFacetAndPosition[lastSelector].functionSelectorPosition = uint96(selectorPosition);

    }

    // delete the last selector

    ds.facetFunctionSelectors[_facetAddress].functionSelectors.pop();

    delete ds.selectorToFacetAndPosition[_selector];



    // if no more selectors for facet address then delete the facet address

    if (lastSelectorPosition == 0) {

        // replace facet address with last facet address and delete last facet address

        uint256 lastFacetAddressPosition = ds.facetAddresses.length - 1;

        uint256 facetAddressPosition = ds.facetFunctionSelectors[_facetAddress].facetAddressPosition;

        if (facetAddressPosition != lastFacetAddressPosition) {

            address lastFacetAddress = ds.facetAddresses[lastFacetAddressPosition];

            ds.facetAddresses[facetAddressPosition] = lastFacetAddress;

            ds.facetFunctionSelectors[lastFacetAddress].facetAddressPosition = facetAddressPosition;

        }

        ds.facetAddresses.pop();

        delete ds.facetFunctionSelectors[_facetAddress].facetAddressPosition;

    }

}