From e250d142a8892eb23bd4082a137240ce4b86a80c Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Tue, 21 Jun 2022 20:02:01 +0800 Subject: [PATCH 1/2] Added validation for negative block number and gas --- .../Validators/HeaderValidatorTests.cs | 30 +++++++++++++++++++ .../Validators/HeaderValidator.cs | 29 ++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/src/Nethermind/Nethermind.Blockchain.Test/Validators/HeaderValidatorTests.cs b/src/Nethermind/Nethermind.Blockchain.Test/Validators/HeaderValidatorTests.cs index 16b32069194..6a513856fd8 100644 --- a/src/Nethermind/Nethermind.Blockchain.Test/Validators/HeaderValidatorTests.cs +++ b/src/Nethermind/Nethermind.Blockchain.Test/Validators/HeaderValidatorTests.cs @@ -258,5 +258,35 @@ public void When_gas_limit_is_long_max_value() Assert.True(result); } + + [Test] + public void When_block_number_is_negative() + { + _block.Header.Number = -1; + _block.Header.Hash = _block.CalculateHash(); + + bool result = _validator.Validate(_block.Header); + Assert.False(result); + } + + [Test] + public void When_gas_used_is_negative() + { + _block.Header.GasUsed = -1; + _block.Header.Hash = _block.CalculateHash(); + + bool result = _validator.Validate(_block.Header); + Assert.False(result); + } + + [Test] + public void When_gas_limit_is_negative() + { + _block.Header.GasLimit = -1; + _block.Header.Hash = _block.CalculateHash(); + + bool result = _validator.Validate(_block.Header); + Assert.False(result); + } } } diff --git a/src/Nethermind/Nethermind.Consensus/Validators/HeaderValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/HeaderValidator.cs index 78f27dc2669..e9854875179 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/HeaderValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/HeaderValidator.cs @@ -63,6 +63,11 @@ public HeaderValidator( /// public virtual bool Validate(BlockHeader header, BlockHeader? parent, bool isUncle = false) { + if (!ValidateFieldLimit(header)) + { + return false; + } + bool hashAsExpected = ValidateHash(header); if (!hashAsExpected) @@ -148,6 +153,30 @@ public virtual bool Validate(BlockHeader header, BlockHeader? parent, bool isUnc extraDataValid && eip1559Valid; } + + private bool ValidateFieldLimit(BlockHeader blockHeader) + { + if (blockHeader.Number < 0) + { + if (_logger.IsWarn) _logger.Warn($"Invalid block header ({blockHeader.Hash}) - Block number is negative {blockHeader.Number}"); + return false; + } + + if (blockHeader.GasLimit < 0) + { + if (_logger.IsWarn) _logger.Warn($"Invalid block header ({blockHeader.Hash}) - Block GasLimit is negative {blockHeader.GasLimit}"); + return false; + } + + if (blockHeader.GasUsed < 0) + { + if (_logger.IsWarn) _logger.Warn($"Invalid block header ({blockHeader.Hash}) - Block GasUsed is negative {blockHeader.GasUsed}"); + return false; + } + + return true; + } + protected virtual bool ValidateExtraData(BlockHeader header, BlockHeader? parent, IReleaseSpec spec, bool isUncle = false) { bool extraDataValid = header.ExtraData.Length <= spec.MaximumExtraDataSize From 1d88c60672933c1cf493acca534a200cf396396c Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Tue, 21 Jun 2022 20:08:06 +0800 Subject: [PATCH 2/2] Added a bit more comment --- .../Nethermind.Consensus/Validators/HeaderValidator.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Nethermind/Nethermind.Consensus/Validators/HeaderValidator.cs b/src/Nethermind/Nethermind.Consensus/Validators/HeaderValidator.cs index e9854875179..f14facce0d2 100644 --- a/src/Nethermind/Nethermind.Consensus/Validators/HeaderValidator.cs +++ b/src/Nethermind/Nethermind.Consensus/Validators/HeaderValidator.cs @@ -156,6 +156,9 @@ public virtual bool Validate(BlockHeader header, BlockHeader? parent, bool isUnc private bool ValidateFieldLimit(BlockHeader blockHeader) { + // Note, these are out of spec. Technically, there could be a block with field with very high value that is + // valid when using ulong, but wrapped to negative value when using long. However, switching to ulong + // at this point can cause other unexpected error. So we just won't support it for now. if (blockHeader.Number < 0) { if (_logger.IsWarn) _logger.Warn($"Invalid block header ({blockHeader.Hash}) - Block number is negative {blockHeader.Number}");