Skip to content

Commit

Permalink
Eof/valdiation fixes (#7405)
Browse files Browse the repository at this point in the history 
Co-authored-by: Ben Adams <thundercat@illyriad.co.uk>
  • Loading branch information
@shemnon @benaadams
shemnon and benaadams authored Sep 10, 2024
1 parent 35d724e commit 42f502a
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/Nethermind/Nethermind.Evm/EvmObjectFormat/EofCodeValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public static bool IsValidEofHeader(ReadOnlyMemory<byte> 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;
Expand Down
79 changes: 74 additions & 5 deletions src/Nethermind/Nethermind.Evm/EvmObjectFormat/Handlers/EofV1.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<byte> 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<byte> containerMemory, ValidationStrategy validationStrategy, out EofHeader? header)
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
static ushort GetUInt16(ReadOnlySpan<byte> container, int offset) =>
Expand All @@ -133,6 +136,11 @@ static ushort GetUInt16(ReadOnlySpan<byte> 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))
{
Expand All @@ -159,6 +167,12 @@ static ushort GetUInt16(ReadOnlySpan<byte> 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");
Expand All @@ -175,6 +189,12 @@ static ushort GetUInt16(ReadOnlySpan<byte> 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");
Expand Down Expand Up @@ -220,12 +240,24 @@ static ushort GetUInt16(ReadOnlySpan<byte> 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");
Expand Down Expand Up @@ -265,6 +297,12 @@ static ushort GetUInt16(ReadOnlySpan<byte> 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");
Expand Down Expand Up @@ -308,6 +346,18 @@ static ushort GetUInt16(ReadOnlySpan<byte> 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,
Expand All @@ -322,7 +372,7 @@ static ushort GetUInt16(ReadOnlySpan<byte> container, int offset) =>

public bool TryGetEofContainer(ReadOnlyMemory<byte> 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;
Expand Down Expand Up @@ -373,7 +423,7 @@ private bool ValidateContainer(EofContainer eofContainer, ValidationStrategy val
continue;

ReadOnlyMemory<byte> 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;
Expand Down Expand Up @@ -553,6 +603,12 @@ private bool ValidateInstructions(EofContainer eofContainer, int sectionId, Vali
{
ReadOnlySpan<byte> 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<byte>.Shared.Rent(length);
byte[] jumpDestsArray = ArrayPool<byte>.Shared.Rent(length);
Expand All @@ -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)
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -1019,7 +1081,14 @@ public bool ValidateStackState(int sectionId, in ReadOnlySpan<byte> 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
{
Expand Down
2 changes: 1 addition & 1 deletion src/Nethermind/Nethermind.Evm/EvmObjectFormat/IEofVersionHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
namespace Nethermind.Evm.EvmObjectFormat;
interface IEofVersionHandler
{
bool TryParseEofHeader(ReadOnlyMemory<byte> code, [NotNullWhen(true)] out EofHeader? header);
bool TryParseEofHeader(ReadOnlyMemory<byte> code, ValidationStrategy strategy, [NotNullWhen(true)] out EofHeader? header);
bool TryGetEofContainer(ReadOnlyMemory<byte> code, ValidationStrategy strategy, [NotNullWhen(true)] out EofContainer? header);
}
5 changes: 3 additions & 2 deletions src/Nethermind/Nethermind.Evm/Instruction.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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),
Expand Down

0 comments on commit 42f502a

Please sign in to comment.