Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require reasons strings can be shortened #31

Closed
code423n4 opened this issue Nov 13, 2021 · 1 comment
Closed

Require reasons strings can be shortened #31

code423n4 opened this issue Nov 13, 2021 · 1 comment
Labels
bug Something isn't working duplicate This issue or pull request already exists G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

Handle

GiveMeTestEther

Vulnerability details

Impact

Reason strings for reverts that don't fit into 32 bytes will increase contract creation gas and and also the gas during runtime if a require condition is not met (at least one additional mstore + othe overhead).

Proof of Concept

2021-11-nested\contracts\FeeSplitter.sol: row 74: require(_msgSender() == weth, "FeeSplitter: ETH_SENDER_NOT_WETH");
2021-11-nested\contracts\FeeSplitter.sol: row 157: require(_royaltiesTarget != address(0), "FeeSplitter: INVALID_ROYALTIES_TARGET_ADDRESS");
2021-11-nested\contracts\FeeSplitter.sol: row 167: require(_accountIndex + 1 <= shareholders.length, "FeeSplitter: INVALID_ACCOUNT_INDEX");
2021-11-nested\contracts\NestedAsset.sol: row 34: require(supportedFactories[_msgSender()], "NestedAsset: FORBIDDEN_NOT_FACTORY");
2021-11-nested\contracts\NestedAsset.sol: row 40: require(_address == ownerOf(_tokenId), "NestedAsset: FORBIDDEN_NOT_OWNER");
2021-11-nested\contracts\NestedAsset.sol: row 115: require(bytes(tokenURI(_tokenId)).length == 0, "NestedAsset: TOKEN_URI_IMMUTABLE");
2021-11-nested\contracts\NestedAsset.sol: row 142: require(supportedFactories[_factory] == true, "NestedAsset: ALREADY_NOT_SUPPORTED");
2021-11-nested\contracts\NestedBuybacker.sol: row 52: require(_burnPercentage <= 1000, "NestedBuybacker::constructor: Burn part to high");
2021-11-nested\contracts\NestedBuybacker.sol: row 82: require(_burnPercentage <= 1000, "NestedBuybacker::setBurnPart: Burn part to high");
2021-11-nested\contracts\NestedFactory.sol: row 56: require(nestedAsset.ownerOf(_nftId) == _msgSender(), "NestedFactory: Not the token owner");
2021-11-nested\contracts\NestedFactory.sol: row 64: require(block.timestamp > nestedRecords.getLockTimestamp(_nftId), "NestedFactory: The NFT is currently locked");
2021-11-nested\contracts\NestedFactory.sol: row 90: require(i > 0, "NestedFactory::removeOperator: Cant remove non-existent operator");
2021-11-nested\contracts\NestedFactory.sol: row 97: require(address(reserve) == address(0), "NestedFactory::setReserve: Reserve is immutable");
2021-11-nested\contracts\NestedFactory.sol: row 104: require(address(_feeSplitter) != address(0), "NestedFactory::setFeeSplitter: Invalid feeSplitter address");
2021-11-nested\contracts\NestedFactory.sol: row 117: require(_orders.length > 0, "NestedFactory::create: Missing orders");
2021-11-nested\contracts\NestedFactory.sol: row 134: require(_orders.length > 0, "NestedFactory::addTokens: Missing orders");
2021-11-nested\contracts\NestedFactory.sol: row 150: require(_orders.length > 0, "NestedFactory::swapTokenForTokens: Missing orders");
2021-11-nested\contracts\NestedFactory.sol: row 169: require(_orders.length > 0, "NestedFactory::sellTokensToNft: Missing orders");
2021-11-nested\contracts\NestedFactory.sol: row 170: require(_sellTokensAmount.length == _orders.length, "NestedFactory::sellTokensToNft: Input lengths must match");
2021-11-nested\contracts\NestedFactory.sol: row 189: require(_orders.length > 0, "NestedFactory::sellTokensToWallet: Missing orders");
2021-11-nested\contracts\NestedFactory.sol: row 220: require(_orders.length > 0, "NestedFactory::destroy: Missing orders");
2021-11-nested\contracts\NestedFactory.sol: row 221: require(tokens.length == _orders.length, "NestedFactory::destroy: Missing sell args");
2021-11-nested\contracts\NestedFactory.sol: row 262: require(assetTokensLength > _tokenIndex, "NestedFactory::withdraw: Invalid token index");
2021-11-nested\contracts\NestedFactory.sol: row 264: require(assetTokensLength > 1, "NestedFactory::withdraw: Can't withdraw the last asset");
2021-11-nested\contracts\NestedFactory.sol: row 391: require(success, "NestedFactory::_submitOrder: Operator call failed");
2021-11-nested\contracts\NestedFactory.sol: row 470: require(holding.amount >= _inputTokenAmount, "NestedFactory:_transferInputTokens: Insufficient amount");
2021-11-nested\contracts\NestedFactory.sol: row 475: require(msg.value == _inputTokenAmount, "NestedFactory::_transferInputTokens: Insufficient amount in");
2021-11-nested\contracts\NestedRecords.sol: row 171: require(_maxHoldingsCount > 0, "NestedRecords: INVALID_MAX_HOLDINGS");
2021-11-nested\contracts\OperatorResolver.sol: row 43: require(names.length == destinations.length, "OperatorResolver::importOperators: Input lengths must match");
2021-11-nested\contracts\libraries\OperatorHelpers.sol: row 51: require(tokens[0] == _outputToken, "OperatorHelpers::getDecodeDataAndRequire: Wrong output token");
2021-11-nested\contracts\libraries\OperatorHelpers.sol: row 52: require(tokens[1] == _inputToken, "OperatorHelpers::getDecodeDataAndRequire: Wrong input token");
2021-11-nested\contracts\operators\Flat\FlatOperator.sol: row 18: require(amount > 0, "FlatOperator::commitAndRevert: Amount must be greater than zero");
2021-11-nested\contracts\operators\ZeroEx\ZeroExOperator.sol: row 38: require(success, "ZeroExOperator::commitAndRevert: 0x swap failed");

Tools Used

Python script

Recommended Mitigation Steps

Shorten the reason strings to max 32 bytes

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Nov 13, 2021
code423n4 added a commit that referenced this issue Nov 13, 2021

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@maximebrugel maximebrugel added the duplicate This issue or pull request already exists label Nov 18, 2021
@maximebrugel
Copy link
Collaborator

Duplicated : #14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

2 participants