search

code-423n4 / 2021-11-unlock-findings

0 stars 0 forks source link

Function grantKeys() - Bulk Send Free Keys Are Not Practical & Gas May Over Block Size Limit #147

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

Meta0xNull

Vulnerability details

Impact

ETH Block Size: The London Upgrade introduced variable-size blocks to Ethereum. Each block has a target size of 15 million gas but, the size of blocks will increase or decrease in accordance with network demand, up until the block limit of 30 million gas (2x the target block size).

Example of ERC721 Creation Gas Usage: 162,642 https://etherscan.io/tx/0x18e9cf8322c8d9a96e7bc9fdacd254f2e4ffbd5f733fb32357bda26e86a3643a

At the end of grantKeys() loop, there is a transfer/creation of ERC721.

When talk about bulk send, I believe we are talking about at least 100.

Gas Cost for 100 Times for simple ERC721 Creation = 162,642 * 100 = 16,264,200 Gas

A simple 100 Times ERC721 Creation Cost 16 Million Gas and it is over ETH Block Size Limit.

Proof of Concept

https://github.com/code-423n4/2021-11-unlock/blob/main/smart-contracts/contracts/mixins/MixinGrantKeys.sol#L22-L55

Tools Used

Manual Review

Recommended Mitigation Steps

Suggest Use ENS Airdrop Method - Merkle Tree. This allow Users to Claim the Free Keys.

More Info: https://medium.com/hackernoon/evolution-of-airdrop-from-common-spam-to-the-merkle-tree-30caa2344170

julien51 commented 2 years ago

We are well aware that there is a "maximum" number of keys that can be granted "at once". This number os actually different on each chain. We believe the front-end should be the place where the appropriate number of keys is granted at once (and if there are more, transactions should be paginated)

0xleastwood commented 2 years ago

I don't think this is too much of an issue. Not only is the sponsor aware, but the call to grant keys can actually be broken up into multiple transactions if required. However, this is something worth noting, so I'll mark it as non-critical.