From 0c876752f6499e6b7015ac0a70ecc357d830bc61 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Tue, 19 Dec 2023 15:18:32 +0100 Subject: [PATCH 1/4] fix memory guard and slice --- .../Tracing/GethStyle/JavaScript/Engine.cs | 10 +++---- .../Nethermind.Evm/Tracing/TraceMemory.cs | 29 +++++++++---------- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/Nethermind/Nethermind.Evm/Tracing/GethStyle/JavaScript/Engine.cs b/src/Nethermind/Nethermind.Evm/Tracing/GethStyle/JavaScript/Engine.cs index 1b5e26d2ba3..4ff2da94a46 100644 --- a/src/Nethermind/Nethermind.Evm/Tracing/GethStyle/JavaScript/Engine.cs +++ b/src/Nethermind/Nethermind.Evm/Tracing/GethStyle/JavaScript/Engine.cs @@ -135,13 +135,11 @@ public Engine(IReleaseSpec spec) private ITypedArray Slice(object input, long start, long end) { ArgumentNullException.ThrowIfNull(input); + var bytes = input.ToBytes(); - if (start < 0 || end < start || end > Array.MaxLength) - { - throw new ArgumentOutOfRangeException(nameof(start), $"tracer accessed out of bound memory: offset {start}, end {end}"); - } - - return input.ToBytes().Slice((int)start, (int)(end - start)).ToTypedScriptArray(); + return start < 0 || end < start || end > bytes.Length + ? throw new ArgumentOutOfRangeException(nameof(start), $"tracer accessed out of bound memory: available {bytes.Length}, offset {start}, size {end - start}") + : bytes.Slice((int)start, (int)(end - start)).ToTypedScriptArray(); } /// diff --git a/src/Nethermind/Nethermind.Evm/Tracing/TraceMemory.cs b/src/Nethermind/Nethermind.Evm/Tracing/TraceMemory.cs index cec5cf8726e..0e0873f29c9 100644 --- a/src/Nethermind/Nethermind.Evm/Tracing/TraceMemory.cs +++ b/src/Nethermind/Nethermind.Evm/Tracing/TraceMemory.cs @@ -45,21 +45,23 @@ public string[] ToHexWordList() return memory; } + private const int MemoryPadLimit = 1024 * 1024; public ReadOnlySpan Slice(int start, int length) { - if ((ulong)start + (ulong)length > Size) - { - throw new IndexOutOfRangeException("Requested memory range is out of bounds."); - } + ArgumentOutOfRangeException.ThrowIfNegative(start, nameof(start)); + ArgumentOutOfRangeException.ThrowIfNegative(length, nameof(length)); ReadOnlySpan span = _memory.Span; - if (start + length > _memory.Length) + if (start + length > span.Length) { + int paddingNeeded = start + length - span.Length; + if (paddingNeeded > MemoryPadLimit) throw new InvalidOperationException($"reached limit for padding memory slice: {paddingNeeded}"); byte[] result = new byte[length]; - for (int i = 0, index = start; index < _memory.Length; i++, index++) + int overlap = span.Length - start; + if (overlap > 0) { - result[i] = span[index]; + span.Slice(start, overlap).CopyTo(result.AsSpan(0, overlap)); } return result; @@ -68,13 +70,8 @@ public ReadOnlySpan Slice(int start, int length) return span.Slice(start, length); } - public BigInteger GetUint(int offset) - { - if (offset < 0 || (ulong)(offset + EvmPooledMemory.WordSize) > Size) - { - throw new ArgumentOutOfRangeException(nameof(offset), $"tracer accessed out of bound memory: available {Size}, offset {offset}, size {EvmPooledMemory.WordSize}"); - } - - return new BigInteger(Slice(offset, EvmPooledMemory.WordSize), true, true); - } + public BigInteger GetUint(int offset) => + offset < 0 || (ulong)(offset + EvmPooledMemory.WordSize) > Size + ? throw new ArgumentOutOfRangeException(nameof(offset), $"tracer accessed out of bound memory: available {Size}, offset {offset}, size {EvmPooledMemory.WordSize}") + : new BigInteger(Slice(offset, EvmPooledMemory.WordSize), true, true); } From 0f1ac037513c8bb5a0f7e4927b2b077736872713 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Thu, 21 Dec 2023 10:51:02 +0100 Subject: [PATCH 2/4] Trim or pad variable length bytes (as Geth does) --- src/Nethermind/Nethermind.Core/Address.cs | 11 +++++++++-- .../GethStyle/JavaScript/JavaScriptConverter.cs | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Nethermind/Nethermind.Core/Address.cs b/src/Nethermind/Nethermind.Core/Address.cs index 3edfee8d79f..9f724f50332 100644 --- a/src/Nethermind/Nethermind.Core/Address.cs +++ b/src/Nethermind/Nethermind.Core/Address.cs @@ -91,13 +91,20 @@ public static bool TryParse(string? value, out Address? address) /// /// Parses string value to Address. String can be shorter than 20 bytes long, it is padded with leading 0's then. /// - public static bool TryParseVariableLength(string? value, out Address? address) + public static bool TryParseVariableLength(string? value, out Address? address, bool allowDifferentSize = false) { if (value is not null) { try { - address = new Address(Extensions.Bytes.FromHexString(value, Size)); + byte[] bytes = Extensions.Bytes.FromHexString(value, Size); + + address = bytes.Length switch + { + > Size when allowDifferentSize => new Address(bytes.Slice(bytes.Length - Size, Size)), + < Size when allowDifferentSize => new Address(bytes.PadLeft(Size)), + _ => new Address(bytes), + }; return true; } catch (IndexOutOfRangeException) { } diff --git a/src/Nethermind/Nethermind.Evm/Tracing/GethStyle/JavaScript/JavaScriptConverter.cs b/src/Nethermind/Nethermind.Evm/Tracing/GethStyle/JavaScript/JavaScriptConverter.cs index 2523f4764c6..9d455e5098a 100644 --- a/src/Nethermind/Nethermind.Evm/Tracing/GethStyle/JavaScript/JavaScriptConverter.cs +++ b/src/Nethermind/Nethermind.Evm/Tracing/GethStyle/JavaScript/JavaScriptConverter.cs @@ -35,7 +35,7 @@ public static class JavaScriptConverter public static Address ToAddress(this object address) => address switch { - string hexString => Address.TryParseVariableLength(hexString, out Address parsedAddress) + string hexString => Address.TryParseVariableLength(hexString, out Address parsedAddress, true) ? parsedAddress : throw new ArgumentException("Not correct address", nameof(address)), _ => new Address(address.ToBytes()) From 0428debeac98189a01b94b08af0eb86f6df27590 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Fri, 22 Dec 2023 17:06:22 +0100 Subject: [PATCH 3/4] fix ArgumentException --- .../Nethermind.Core.Test/AddressTests.cs | 21 +++++++++++++ src/Nethermind/Nethermind.Core/Address.cs | 31 ++++++++++++------- .../Nethermind.Core/Extensions/Bytes.cs | 2 +- .../Nethermind.Evm/Tracing/TraceMemory.cs | 2 +- 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/Nethermind/Nethermind.Core.Test/AddressTests.cs b/src/Nethermind/Nethermind.Core.Test/AddressTests.cs index 93b2274d08e..b286e202187 100644 --- a/src/Nethermind/Nethermind.Core.Test/AddressTests.cs +++ b/src/Nethermind/Nethermind.Core.Test/AddressTests.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: LGPL-3.0-only using System.Collections; +using FluentAssertions; using Nethermind.Blockchain; using Nethermind.Core.Crypto; using Nethermind.Core.Extensions; @@ -183,6 +184,26 @@ public void Of_contract(long nonce, string expectedAddress) public bool Is_PointEvaluationPrecompile_properly_activated(IReleaseSpec spec) => Address.FromNumber(0x0a).IsPrecompile(spec); + [TestCase(Address.SystemUserHex, false)] + [TestCase("2" + Address.SystemUserHex, false)] + [TestCase("2" + Address.SystemUserHex, true)] + public void Parse_variable_length(string addressHex, bool allowOverflow) + { + var result = Address.TryParseVariableLength(addressHex, out Address? address, allowOverflow); + result.Should().Be(addressHex.Length <= Address.SystemUserHex.Length || allowOverflow); + if (result) + { + address.Should().Be(Address.SystemUser); + } + } + + [Test] + public void Parse_variable_length_too_short() + { + Address.TryParseVariableLength("1", out Address? address).Should().Be(true); + address.Should().Be(new Address("0000000000000000000000000000000000000001")); + } + public static IEnumerable PointEvaluationPrecompileTestCases { get diff --git a/src/Nethermind/Nethermind.Core/Address.cs b/src/Nethermind/Nethermind.Core/Address.cs index 9f724f50332..81b0787d279 100644 --- a/src/Nethermind/Nethermind.Core/Address.cs +++ b/src/Nethermind/Nethermind.Core/Address.cs @@ -24,7 +24,8 @@ public class Address : IEquatable
, IComparable
private const int PrefixedHexCharsCount = 2 + HexCharsCount; // 0x5a4eab120fb44eb6684e5e32785702ff45ea344d public static Address Zero { get; } = new(new byte[Size]); - public static Address SystemUser { get; } = new("0xfffffffffffffffffffffffffffffffffffffffe"); + public const string SystemUserHex = "0xfffffffffffffffffffffffffffffffffffffffe"; + public static Address SystemUser { get; } = new(SystemUserHex); public byte[] Bytes { get; } @@ -91,25 +92,31 @@ public static bool TryParse(string? value, out Address? address) /// /// Parses string value to Address. String can be shorter than 20 bytes long, it is padded with leading 0's then. /// - public static bool TryParseVariableLength(string? value, out Address? address, bool allowDifferentSize = false) + public static bool TryParseVariableLength(string? value, out Address? address, bool allowOverflow = false) { if (value is not null) { - try - { - byte[] bytes = Extensions.Bytes.FromHexString(value, Size); + const int size = Size << 1; - address = bytes.Length switch + int start = value is ['0', 'x', ..] ? 2 : 0; + ReadOnlySpan span = value.AsSpan(start); + if (span.Length > size) + { + if (allowOverflow) + { + span = span.Slice(value.Length - size); + } + else { - > Size when allowDifferentSize => new Address(bytes.Slice(bytes.Length - Size, Size)), - < Size when allowDifferentSize => new Address(bytes.PadLeft(Size)), - _ => new Address(bytes), - }; - return true; + goto False; + } } - catch (IndexOutOfRangeException) { } + + address = new Address(Extensions.Bytes.FromHexString(span, Size)); + return true; } + False: address = default; return false; } diff --git a/src/Nethermind/Nethermind.Core/Extensions/Bytes.cs b/src/Nethermind/Nethermind.Core/Extensions/Bytes.cs index 7978ad40043..c772a39b766 100644 --- a/src/Nethermind/Nethermind.Core/Extensions/Bytes.cs +++ b/src/Nethermind/Nethermind.Core/Extensions/Bytes.cs @@ -1018,7 +1018,7 @@ public static byte[] FromHexString(string hexString, int length) => hexString is null ? throw new ArgumentNullException(nameof(hexString)) : FromHexString(hexString.AsSpan(), length); [DebuggerStepThrough] - private static byte[] FromHexString(ReadOnlySpan hexString, int length) + public static byte[] FromHexString(ReadOnlySpan hexString, int length) { int start = hexString is ['0', 'x', ..] ? 2 : 0; ReadOnlySpan chars = hexString.Slice(start); diff --git a/src/Nethermind/Nethermind.Evm/Tracing/TraceMemory.cs b/src/Nethermind/Nethermind.Evm/Tracing/TraceMemory.cs index 0e0873f29c9..cc63b903019 100644 --- a/src/Nethermind/Nethermind.Evm/Tracing/TraceMemory.cs +++ b/src/Nethermind/Nethermind.Evm/Tracing/TraceMemory.cs @@ -21,7 +21,7 @@ public TraceMemory(ulong size, ReadOnlyMemory memory) public string[] ToHexWordList() { - string[] memory = new string[((int)Size / EvmPooledMemory.WordSize) + ((Size % EvmPooledMemory.WordSize == 0) ? 0 : 1)]; + string[] memory = new string[(int)Size / EvmPooledMemory.WordSize + (Size % EvmPooledMemory.WordSize == 0 ? 0 : 1)]; int traceLocation = 0; int i = 0; From 2f5da2b0b62032de67d70a6fb9bcedbf207bdfdd Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Fri, 22 Dec 2023 22:49:10 +0100 Subject: [PATCH 4/4] whitespace --- src/Nethermind/Nethermind.Core/Address.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Core/Address.cs b/src/Nethermind/Nethermind.Core/Address.cs index 81b0787d279..98b1e919b72 100644 --- a/src/Nethermind/Nethermind.Core/Address.cs +++ b/src/Nethermind/Nethermind.Core/Address.cs @@ -116,7 +116,7 @@ public static bool TryParseVariableLength(string? value, out Address? address, b return true; } - False: + False: address = default; return false; }