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

Add a size limit to block_cache_unverified. #1859

Closed
ShawnYun opened this issue Aug 20, 2020 · 0 comments · Fixed by #1870
Closed

Add a size limit to block_cache_unverified. #1859

ShawnYun opened this issue Aug 20, 2020 · 0 comments · Fixed by #1870
Labels
discussion Initial issue state - proposed but not yet accepted

Comments

@ShawnYun
Copy link
Contributor

ShawnYun commented Aug 20, 2020

Summary or problem description

If we received a block, OnNewBlock will be called to add the block.

321         private VerifyResult OnNewBlock(Block block)
322         {
323             if (block.Index <= Height)
324                 return VerifyResult.AlreadyExists;
325             if (block.Index - 1 > Height)
326             {
327                 AddUnverifiedBlockToCache(block);
328                 return VerifyResult.UnableToVerify;
329             }
330             if (block.Index == Height + 1)
331             {
332                 if (!block.Verify(currentSnapshot))
333                     return VerifyResult.Invalid;
334                 block_cache.TryAdd(block.Hash, block);
335                 block_cache_unverified.Remove(block.Index);
336                 // We can store the new block in block_cache and tell the new height to other nodes before Persist().
337                 system.LocalNode.Tell(Message.Create(MessageCommand.Ping, PingPayload.Create(Singleton.Height + 1)));
338                 Persist(block);
339                 SaveHeaderHashList();
340                 if (block_cache_unverified.TryGetValue(Height + 1, out LinkedList<Block> unverifiedBlocks))
341                 {
342                     foreach (var unverifiedBlock in unverifiedBlocks)
343                         Self.Tell(unverifiedBlock, ActorRefs.NoSender);
344                     block_cache_unverified.Remove(Height + 1);
345                 }
346             }
347             return VerifyResult.Succeed;
348         }

If the block index is bigger than current block height plus 1, it will be added to unverified cache directly by calling AddUnverifiedBlockToCache.

Specifically, an attacker can keep sending blocks with large block index to flood the block_cache_unverified and cause the node Out-of-Memory. Further more, since the block_cache_unverified stores blocks with same index but different hashes into a LinkedList. Attacker can also flood the node with blocks that has same index but different hashes.

Do you have any solution you want to propose?

  1. Add a size limit to block_cache_unverified.

Neo Version

  • Neo 3

Where in the software does this update applies to?

  • Ledger
  • P2P (TCP)
@ShawnYun ShawnYun added the discussion Initial issue state - proposed but not yet accepted label Aug 20, 2020
@ShawnYun ShawnYun changed the title Add check when received a block, and add a size limit to block_cache_unverified. Add a size limit to block_cache_unverified. Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Initial issue state - proposed but not yet accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant