fix: cap compact log entry zero-prefix data expansion

[JayeTurn]

Changes

• Reject compact log entries whose decoded zero-prefix expansion would exceed the existing LogEntry 16 MB RLP limit
• Apply the same bound in both Decode and DecodeLogEntryStructRef so malformed receipts cannot trigger oversized heap allocations
• Add regression tests covering both compact log entry decode paths

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)
• New feature (a non-breaking change that adds functionality)
• Breaking change (a change that causes existing functionality not to work as expected)
• Optimization
• Refactoring
• Documentation update
• Build-related changes
• Other: Description

Testing

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

Notes on testing

• dotnet test --project src/Nethermind/Nethermind.Core.Test/Nethermind.Core.Test.csproj -c release --filter FullyQualifiedName~LogEntryDecoderTests

Documentation

Requires documentation update

• Yes
• No

Requires explanation in Release Notes

• Yes
• No

[asdacap]

Isn this whta the decoderContext.SkipItem() do?

[asdacap]

Sorry, I mean decoderContext.Check().

[alexb5dh]

Don't think Check can help here, but using

Rlp.GuardLimit(zeroPrefix, RlpLimit.Limit - rlpData.Length, RlpLimit);

will make it follow general approach of hiding exception behind an Rlp* method. Negativity check will soon be removed anyway, as all DecodeInt will be replaced with DecodeUInt.

Alternatively, just moving exception-throwing to RlpHelper should also work.

[LukaszRozmej]

Review Summary

Clean, well-scoped fix. The approach — validating the decoded zeroPrefix against the existing LogEntry 16 MB limit before allocating — is correct, and extracting DecodeCompactData cleanly deduplicates the two call sites. Regression tests cover both paths.

Good

• The bound check zeroPrefix > RlpLimit.Limit - rlpData.Length is overflow-safe: it avoids computing zeroPrefix + rlpData.Length (which would wrap for large zeroPrefix) and, because rlpData.Length >= 0, the RHS is always <= RlpLimit.Limit, so it also correctly rejects rlpData.Length values that on their own exceed the limit.
• The zeroPrefix < 0 guard is necessary — DecodeInt returns a signed int, and a crafted RLP could produce a negative value that would otherwise bypass the upper-bound comparison (via negative arithmetic) or cause a negative allocation attempt.
• Tests exercise both Decode and DecodeLogEntryStructRef, hitting the two places the same helper is now used.

Minor observations (non-blocking)

1. DecodeLogEntryStructRef does not call GuardLimit(logEntryLength, RlpLimit) the way Decode does at line 27. That's pre-existing and out of scope here, but the asymmetry is worth noting — the struct-ref path still relies solely on the new DecodeCompactData check to bound the data buffer; the topics region is only bounded by the surrounding RLP. Consider a follow-up that mirrors the GuardLimit(logEntryLength, RlpLimit) call for consistency.

2. Local dataLength in DecodeCompactData is used once — it could be inlined (new byte[zeroPrefix + rlpData.Length]). Minor stylistic nit only; safe because the preceding check guarantees the sum fits in int without overflow.

3. Test helper name CreateCompactLogEntryWithTooLargeZeroPrefix is accurate, but strictly the payload is encoded with the standard RLP encoder — not via CompactLogEntryDecoder.Encode. That's fine and intentional (it's meant to be malformed), but a brief comment clarifying the wire format shape ([address, topics, zeroPrefix, rlpData]) would help future readers understand what's being simulated.

Nothing else flagged

• No impact on the encode path.
• The happy-path roundtrip tests already in LogEntryDecoderTests continue to exercise the non-malicious compact encoding via ReceiptDecoder / receipt storage paths — no need to duplicate.

LGTM after (or without) addressing the minor observations.

[alexb5dh]

Looks ok, but not critical, as only used for decoding from Receipts DB.

Agree on addressing #​1 and #​2 from minor observations.

[alexb5dh]

Don't think Check can help here, but using

Rlp.GuardLimit(zeroPrefix, RlpLimit.Limit - rlpData.Length, RlpLimit);

will make it follow general approach of hiding exception behind an Rlp* method. Negativity check will soon be removed anyway, as all DecodeInt will be replaced with DecodeUInt.

Alternatively, just moving exception-throwing to RlpHelper should also work.

[JayeTurn]

Makes sense. I'll switch this to the Rlp.GuardLimit(...) style and also address #1 and #2.

[JayeTurn]

Applied the guard changes and added a regression test. Let me know if anything still looks off. Thanks for your review!

[LukaszRozmej]

@claude review

[claude]

Claude finished @LukaszRozmej's task in 3m 56s —— View job

---

Code Review

• Gather context and understand request
• Read changed files
• Read relevant coding rules
• Conduct review and post findings

---

This is a clean, well-scoped security fix addressing a heap-amplification vulnerability in CompactLogEntryDecoder. Both commits have been reviewed.

What the PR does

CompactLogEntryDecoder stores log data as [zeroPrefix, rlpData] where the original data is reconstructed by prepending zeroPrefix zero bytes to rlpData. Without a guard, a malicious compact log entry could encode a huge zeroPrefix (e.g., int.MaxValue), triggering a ~2 GB allocation.

The fix extracts DecodeCompactData, guards both the negative case and the overflow case, and applies the guard uniformly to both Decode and DecodeLogEntryStructRef.

---

CompactLogEntryDecoder.cs

Commit c3410d3 — initial fix

DecodeCompactData helper correctly deduplicates the two call sites. The original check was:

if (zeroPrefix < 0 || zeroPrefix > RlpLimit.Limit - rlpData.Length)

Commit 4201cb4 — address review observations

Refactors the check to use Rlp.GuardLimit, consistent with the codebase pattern:

if (zeroPrefix < 0)
    throw new RlpLimitException(...);
Rlp.GuardLimit(zeroPrefix, RlpLimit.Limit - rlpData.Length, RlpLimit);

Splitting the checks is correct and necessary. Rlp.GuardLimit only checks count > bytesLeft || count > l.Limit — if zeroPrefix were negative and bytesLeft were also negative (hypothetically), the guard would not fire. The explicit zeroPrefix < 0 pre-check makes the negative case unambiguous. ✓

The RlpLimit.Limit - rlpData.Length expression is overflow-safe: the surrounding GuardLimit(logEntryLength, RlpLimit) on the outer sequence already bounds logEntryLength ≤ 16MB, which transitively bounds rlpData.Length, so the subtraction cannot underflow. ✓

The second commit also adds decoderContext.GuardLimit(logEntryLength, RlpLimit) to DecodeLogEntryStructRef (lines 57), making it symmetric with Decode (line 27). This addresses observation #1 from the prior review. ✓

LogEntryDecoderTests.cs

Three new tests added. Verified .MB uses SI metric (1_000_000), so (int)16.MB = 16_000_000:

Compact_decoder_rejects_zero_prefix_that_expands_data_beyond_limit

• zeroPrefix = 16_000_000, rlpData.Length = 1
• Fires Rlp.GuardLimit(16_000_000, 15_999_999, ...) → 16_000_000 > 15_999_999 → throws ✓

Compact_struct_ref_decoder_rejects_zero_prefix_that_expands_data_beyond_limit

• Same malformed payload through the struct-ref path ✓

Compact_struct_ref_decoder_rejects_log_entry_length_beyond_limit

• CreateCompactLogEntryWithTooLargeDeclaredLength() returns [0xfa, 0xF4, 0x24, 0x01], correctly encoding a 3-byte RLP length prefix for 16_000_001 = 0xF42401 ✓
• GuardLimit(16_000_001, RlpLimit) throws since 16_000_001 > 16_000_000 ✓
• The wire format comment added to CreateCompactLogEntryWithTooLargeZeroPrefix (line 137) addresses observation Netcore #3 ✓

Minor observations (non-blocking)

1. No regression test for negative zeroPrefix: the zeroPrefix < 0 branch is exercised by the existing tests only if DecodeInt() can produce a negative value from crafted RLP. An RLP-encoded -1 passed as the zeroPrefix field would hit this path, but there's no dedicated test for it. Low priority since an attacker targeting heap amplification would use a large positive value, not a negative one.

2. Error message asymmetry: the zeroPrefix < 0 branch reuses the "exceeds limit" message text, which is slightly misleading. The message says "exceeds limit" when the actual issue is that the prefix is negative. This is cosmetic and non-blocking.

---

Summary

All three of @LukaszRozmej's prior observations have been addressed. The fix is correct, the test coverage is solid, and the implementation is consistent with codebase conventions. LGTM.

[LukaszRozmej]

Remove duplication in tests:

// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited
// SPDX-License-Identifier: LGPL-3.0-only

using System;
using Nethermind.Blockchain.Receipts;
using Nethermind.Core.Crypto;
using Nethermind.Core.Extensions;
using Nethermind.Core.Test.Builders;
using Nethermind.Serialization.Rlp;
using NUnit.Framework;

namespace Nethermind.Core.Test.Encoding;

public class LogEntryDecoderTests
{
    private static LogEntry CreateSampleLogEntry() =>
        new(TestItem.AddressA, new byte[] { 1, 2, 3 }, new[] { TestItem.KeccakA, TestItem.KeccakB });

    private static void AssertLogEntriesEqual(LogEntry expected, LogEntry actual)
    {
        Assert.That(actual.Data, Is.EqualTo(expected.Data), "data");
        Assert.That(actual.Address, Is.EqualTo(expected.Address), "address");
        Assert.That(actual.Topics, Is.EqualTo(expected.Topics), "topics");
    }

    [TestCase(true, false)]
    [TestCase(false, false)]
    [TestCase(false, true)]
    public void Can_do_roundtrip(bool valueDecode, bool useDecoderInstance)
    {
        LogEntry logEntry = CreateSampleLogEntry();

        Rlp rlp = useDecoderInstance
            ? LogEntryDecoder.Instance.Encode(logEntry)
            : Rlp.Encode(logEntry);

        LogEntry decoded;
        if (useDecoderInstance)
        {
            Rlp.ValueDecoderContext ctx = new(rlp.Bytes);
            decoded = LogEntryDecoder.Instance.Decode(ref ctx)!;
        }
        else
        {
            decoded = valueDecode
                ? Rlp.Decode<LogEntry>(rlp.Bytes.AsSpan())
                : Rlp.Decode<LogEntry>(rlp);
        }

        AssertLogEntriesEqual(logEntry, decoded);
    }

    [Test]
    public void Can_do_roundtrip_ref_struct()
    {
        LogEntry logEntry = CreateSampleLogEntry();
        Rlp rlp = Rlp.Encode(logEntry);
        Rlp.ValueDecoderContext valueDecoderContext = new(rlp.Bytes);
        LogEntryDecoder.DecodeStructRef(ref valueDecoderContext, RlpBehaviors.None, out LogEntryStructRef decoded);

        Assert.That(Bytes.AreEqual(logEntry.Data, decoded.Data), "data");
        Assert.That(logEntry.Address == decoded.Address, "address");

        Span<byte> buffer = stackalloc byte[32];
        KeccaksIterator iterator = new(decoded.TopicsRlp, buffer);
        for (int i = 0; i < logEntry.Topics.Length; i++)
        {
            iterator.TryGetNext(out Hash256StructRef keccak);
            Assert.That(logEntry.Topics[i] == keccak, $"topics[{i}]");
        }
    }

    [Test]
    public void Can_handle_nulls()
    {
        Rlp rlp = Rlp.Encode((LogEntry)null!);
        LogEntry decoded = Rlp.Decode<LogEntry>(rlp);
        Assert.That(decoded, Is.Null);
    }

    [Test]
    public void Rejects_extra_topic_items_inside_topics_sequence()
    {
        Rlp malformed = Rlp.Encode(
            Rlp.Encode(TestItem.AddressA.Bytes),
            Rlp.Encode(Rlp.Encode(TestItem.KeccakA.Bytes), Rlp.OfEmptyByteArray),
            Rlp.OfEmptyByteArray);

        Assert.Throws<RlpException>(() =>
        {
            Rlp.ValueDecoderContext ctx = new(malformed.Bytes);
            LogEntryDecoder.Instance.Decode(ref ctx);
        });
    }

    [TestCase(false)]
    [TestCase(true)]
    public void Compact_decoder_rejects_zero_prefix_that_expands_data_beyond_limit(bool useStructRef)
    {
        Rlp malformed = CreateCompactLogEntryWithTooLargeZeroPrefix();

        Assert.Throws<RlpLimitException>(() =>
        {
            Rlp.ValueDecoderContext ctx = new(malformed.Bytes);
            if (useStructRef)
                CompactLogEntryDecoder.DecodeLogEntryStructRef(ref ctx, RlpBehaviors.None, out _);
            else
                CompactLogEntryDecoder.Decode(ref ctx);
        });
    }

    [Test]
    public void Compact_struct_ref_decoder_rejects_log_entry_length_beyond_limit()
    {
        byte[] malformed = CreateCompactLogEntryWithTooLargeDeclaredLength();

        Assert.Throws<RlpLimitException>(() =>
        {
            Rlp.ValueDecoderContext ctx = new(malformed);
            CompactLogEntryDecoder.DecodeLogEntryStructRef(ref ctx, RlpBehaviors.None, out _);
        });
    }

    private static Rlp CreateCompactLogEntryWithTooLargeZeroPrefix() => Rlp.Encode(
        Rlp.Encode(TestItem.AddressA.Bytes),
        Rlp.OfEmptyList,
        Rlp.Encode((int)16.MB),
        Rlp.Encode(new byte[] { 1 }));

    private static byte[] CreateCompactLogEntryWithTooLargeDeclaredLength()
    {
        int declaredLength = (int)16.MB + 1;
        return
        [
            0xfa,
            (byte)(declaredLength >> 16),
            (byte)(declaredLength >> 8),
            (byte)declaredLength,
        ];
    }
}