code-423n4 / 2021-05-nftx-findings

1 stars 0 forks source link

Storage variable reads can be hoisted out of loops #70

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

Storage variable reads via SLOADs are much more expensive (2100 gas with EIP-2929 in Berlin) than memory reads via MLOADs (3 gas). If the same storage variable is read during loop iterations, it is 700x gas efficient to hoist it out of the loop, assign it to a memory variable which is instead used inside the loop. While this is done in most places, there are three places where it is missing.

Proof of Concept

is1155: https://github.com/code-423n4/2021-05-nftx/blob/f6d793c136d110774de259d9f3b25d003c4f8098/nftx-protocol-v2/contracts/solidity/eligibility/NFTXMintRequestEligibility.sol#L30 https://github.com/code-423n4/2021-05-nftx/blob/f6d793c136d110774de259d9f3b25d003c4f8098/nftx-protocol-v2/contracts/solidity/eligibility/NFTXMintRequestEligibility.sol#L120

vault: https://github.com/code-423n4/2021-05-nftx/blob/f6d793c136d110774de259d9f3b25d003c4f8098/nftx-protocol-v2/contracts/solidity/eligibility/NFTXMintRequestEligibility.sol#L29 https://github.com/code-423n4/2021-05-nftx/blob/f6d793c136d110774de259d9f3b25d003c4f8098/nftx-protocol-v2/contracts/solidity/eligibility/NFTXMintRequestEligibility.sol#L161 https://github.com/code-423n4/2021-05-nftx/blob/f6d793c136d110774de259d9f3b25d003c4f8098/nftx-protocol-v2/contracts/solidity/eligibility/NFTXMintRequestEligibility.sol#L184

Tools Used

Manual Analysis

Recommended Mitigation Steps

Hoist storage variable reads out of the loop, assign it to a memory variable which is instead used inside the loop.