From 42f502af6ea8b615f733efbd79fbd5950945d0d8 Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Tue, 10 Sep 2024 00:29:11 -0600 Subject: [PATCH] Eof/valdiation fixes (#7405) Co-authored-by: Ben Adams --- .../EvmObjectFormat/EofCodeValidator.cs | 2 +- .../EvmObjectFormat/Handlers/EofV1.cs | 79 +++++++++++++++++-- .../EvmObjectFormat/IEofVersionHandler.cs | 2 +- src/Nethermind/Nethermind.Evm/Instruction.cs | 5 +- 4 files changed, 79 insertions(+), 9 deletions(-) diff --git a/src/Nethermind/Nethermind.Evm/EvmObjectFormat/EofCodeValidator.cs b/src/Nethermind/Nethermind.Evm/EvmObjectFormat/EofCodeValidator.cs index 06185b7acc0..ebcc7f67c6c 100644 --- a/src/Nethermind/Nethermind.Evm/EvmObjectFormat/EofCodeValidator.cs +++ b/src/Nethermind/Nethermind.Evm/EvmObjectFormat/EofCodeValidator.cs @@ -52,7 +52,7 @@ public static bool IsValidEofHeader(ReadOnlyMemory code, [NotNullWhen(true { if (IsEof(code, out byte version) && _eofVersionHandlers.TryGetValue(version, out IEofVersionHandler handler)) { - return handler.TryParseEofHeader(code, out header); + return handler.TryParseEofHeader(code, ValidationStrategy.Validate, out header); } header = null; diff --git a/src/Nethermind/Nethermind.Evm/EvmObjectFormat/Handlers/EofV1.cs b/src/Nethermind/Nethermind.Evm/EvmObjectFormat/Handlers/EofV1.cs index b205803b32d..06d9db617af 100644 --- a/src/Nethermind/Nethermind.Evm/EvmObjectFormat/Handlers/EofV1.cs +++ b/src/Nethermind/Nethermind.Evm/EvmObjectFormat/Handlers/EofV1.cs @@ -118,7 +118,10 @@ internal enum Separator : byte + MINIMUM_CODESECTION_SIZE // minimum code section body size + MINIMUM_DATASECTION_SIZE; // minimum data section body size - public bool TryParseEofHeader(ReadOnlyMemory containerMemory, out EofHeader? header) + // EIP-3540 ties this to MAX_INIT_CODE_SIZE from EIP-3860, but we need a constant here + internal const ushort MAXIMUM_SIZE = 0xc000; + + public bool TryParseEofHeader(ReadOnlyMemory containerMemory, ValidationStrategy validationStrategy, out EofHeader? header) { [MethodImpl(MethodImplOptions.AggressiveInlining)] static ushort GetUInt16(ReadOnlySpan container, int offset) => @@ -133,6 +136,11 @@ static ushort GetUInt16(ReadOnlySpan container, int offset) => if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Code is too small to be valid code"); return false; } + if (container.Length > MAXIMUM_SIZE) + { + if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Code is larger than allowed maximum size of {MAXIMUM_SIZE}"); + return false; + } if (!container.StartsWith(EofValidator.MAGIC)) { @@ -159,6 +167,12 @@ static ushort GetUInt16(ReadOnlySpan container, int offset) => switch (separator) { case Separator.KIND_TYPE: + if (sectionSizes.TypeSectionSize != null) + { + if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Multiple type sections"); + return false; + } + if (container.Length < pos + EofValidator.TWO_BYTE_LENGTH) { if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Code is too small to be valid code"); @@ -175,6 +189,12 @@ static ushort GetUInt16(ReadOnlySpan container, int offset) => pos += EofValidator.TWO_BYTE_LENGTH; break; case Separator.KIND_CODE: + if (sectionSizes.CodeSectionSize != null) + { + if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Multiple code sections"); + return false; + } + if (sectionSizes.TypeSectionSize is null) { if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Code is not well fromated"); @@ -220,12 +240,24 @@ static ushort GetUInt16(ReadOnlySpan container, int offset) => pos += EofValidator.TWO_BYTE_LENGTH + EofValidator.TWO_BYTE_LENGTH * numberOfCodeSections; break; case Separator.KIND_CONTAINER: + if (sectionSizes.ContainerSectionSize != null) + { + if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Multiple container sections"); + return false; + } + if (sectionSizes.CodeSectionSize is null) { if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Code is not well fromated"); return false; } + if (sectionSizes.DataSectionSize is not null) + { + if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Container section is out of order"); + return false; + } + if (container.Length < pos + EofValidator.TWO_BYTE_LENGTH) { if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Code is too small to be valid code"); @@ -265,6 +297,12 @@ static ushort GetUInt16(ReadOnlySpan container, int offset) => pos += EofValidator.TWO_BYTE_LENGTH + EofValidator.TWO_BYTE_LENGTH * numberOfContainerSections; break; case Separator.KIND_DATA: + if (sectionSizes.DataSectionSize != null) + { + if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Multiple data sections"); + return false; + } + if (sectionSizes.CodeSectionSize is null) { if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Code is not well fromated"); @@ -308,6 +346,18 @@ static ushort GetUInt16(ReadOnlySpan container, int offset) => : new CompoundSectionHeader(codeSectionSubHeader.EndOffset, containerSections); var dataSectionSubHeader = new SectionHeader(containerSectionSubHeader?.EndOffset ?? codeSectionSubHeader.EndOffset, sectionSizes.DataSectionSize.Value); + if (dataSectionSubHeader.EndOffset < containerMemory.Length) + { + if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Extra data after end of container, starting at {dataSectionSubHeader.EndOffset}"); + return false; + } + if ((validationStrategy.HasFlag(ValidationStrategy.Validate) && !validationStrategy.HasFlag(ValidationStrategy.ValidateRuntimeMode)) + && dataSectionSubHeader.EndOffset > container.Length) + { + if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Container has truncated data where full data is required"); + return false; + } + header = new EofHeader { Version = VERSION, @@ -322,7 +372,7 @@ static ushort GetUInt16(ReadOnlySpan container, int offset) => public bool TryGetEofContainer(ReadOnlyMemory code, ValidationStrategy validationStrategy, [NotNullWhen(true)] out EofContainer? eofContainer) { - if (!TryParseEofHeader(code, out EofHeader? header)) + if (!TryParseEofHeader(code, validationStrategy, out EofHeader? header)) { eofContainer = null; return false; @@ -373,7 +423,7 @@ private bool ValidateContainer(EofContainer eofContainer, ValidationStrategy val continue; ReadOnlyMemory subsection = targetContainer.ContainerSections[worklet.Index - 1]; - if (!TryParseEofHeader(subsection, out EofHeader? header) || + if (!TryParseEofHeader(subsection, validationStrategy, out EofHeader? header) || !ValidateBody(subsection.Span, header.Value, validationStrategy)) { return false; @@ -553,6 +603,12 @@ private bool ValidateInstructions(EofContainer eofContainer, int sectionId, Vali { ReadOnlySpan code = eofContainer.CodeSections[sectionId].Span; + if (code.Length < 1) + { + if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, CodeSection {sectionId} is too short to be valid"); + return false; + } + var length = code.Length / BYTE_BIT_COUNT + 1; byte[] codeBitmapArray = ArrayPool.Shared.Rent(length); byte[] jumpDestsArray = ArrayPool.Shared.Rent(length); @@ -570,9 +626,10 @@ private bool ValidateInstructions(EofContainer eofContainer, int sectionId, Vali var isCurrentSectionNonReturning = currentTypesection[OUTPUTS_OFFSET] == 0x80; int pos; + Instruction opcode = Instruction.STOP; for (pos = 0; pos < code.Length;) { - var opcode = (Instruction)code[pos]; + opcode = (Instruction)code[pos]; var postInstructionByte = pos + 1; if (opcode is Instruction.RETURN or Instruction.STOP) @@ -810,7 +867,6 @@ private bool ValidateInstructions(EofContainer eofContainer, int sectionId, Vali } var initcodeSectionId = code[postInstructionByte]; - BitmapHelper.HandleNumbits(EofValidator.ONE_BYTE_LENGTH, codeBitmap, ref postInstructionByte); if (eofContainer.Header.ContainerSections is null || initcodeSectionId >= eofContainer.Header.ContainerSections?.Count) { @@ -849,6 +905,12 @@ private bool ValidateInstructions(EofContainer eofContainer, int sectionId, Vali return false; } + if (!opcode.IsTerminating()) + { + if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Code section {sectionId} ends with a non-terminating opcode"); + return false; + } + var result = !BitmapHelper.CheckCollision(codeBitmap, jumpDests); if (!result) if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, Invalid Jump destination"); @@ -1019,7 +1081,14 @@ public bool ValidateStackState(int sectionId, in ReadOnlySpan code, in Rea if (opcode.IsTerminating()) { if (programCounter < code.Length) + { + if (recordedStackHeight[programCounter].Max < 0) + { + if (Logger.IsTrace) Logger.Trace($"EOF: Eof{VERSION}, opcode not forward referenced, section {sectionId} pc {programCounter}"); + return false; + } currentStackBounds = recordedStackHeight[programCounter]; + } } else { diff --git a/src/Nethermind/Nethermind.Evm/EvmObjectFormat/IEofVersionHandler.cs b/src/Nethermind/Nethermind.Evm/EvmObjectFormat/IEofVersionHandler.cs index 407a97dfc3b..7254ced54ea 100644 --- a/src/Nethermind/Nethermind.Evm/EvmObjectFormat/IEofVersionHandler.cs +++ b/src/Nethermind/Nethermind.Evm/EvmObjectFormat/IEofVersionHandler.cs @@ -7,6 +7,6 @@ namespace Nethermind.Evm.EvmObjectFormat; interface IEofVersionHandler { - bool TryParseEofHeader(ReadOnlyMemory code, [NotNullWhen(true)] out EofHeader? header); + bool TryParseEofHeader(ReadOnlyMemory code, ValidationStrategy strategy, [NotNullWhen(true)] out EofHeader? header); bool TryGetEofContainer(ReadOnlyMemory code, ValidationStrategy strategy, [NotNullWhen(true)] out EofContainer? header); } diff --git a/src/Nethermind/Nethermind.Evm/Instruction.cs b/src/Nethermind/Nethermind.Evm/Instruction.cs index 6e17f9ede38..3380e00e2ce 100644 --- a/src/Nethermind/Nethermind.Evm/Instruction.cs +++ b/src/Nethermind/Nethermind.Evm/Instruction.cs @@ -195,7 +195,7 @@ public enum Instruction : byte EXCHANGE = 0xe8, // random value opcode spec has collision RETURNDATALOAD = 0xf7, - // opcode value not spec-ed + // opcode value not spec-ed EXTCALL = 0xf8, EXTDELEGATECALL = 0xf9, // DelegateCallEnabled EXTSTATICCALL = 0xfb, // StaticCallEnabled @@ -240,6 +240,7 @@ public static bool IsValid(this Instruction instruction, bool IsEofContext) Instruction.CALL => !IsEofContext, Instruction.CALLCODE => !IsEofContext, Instruction.DELEGATECALL => !IsEofContext, + Instruction.STATICCALL => !IsEofContext, Instruction.SELFDESTRUCT => !IsEofContext, Instruction.JUMP => !IsEofContext, Instruction.JUMPI => !IsEofContext, @@ -255,7 +256,7 @@ public static bool IsValid(this Instruction instruction, bool IsEofContext) }; } - //Note() : Extensively test this, refactor it, + //Note() : Extensively test this, refactor it, public static (ushort? InputCount, ushort? OutputCount, ushort? immediates) StackRequirements(this Instruction instruction) => instruction switch { Instruction.STOP => (0, 0, 0),