EOF validation updates (#7419)

* Ensure forward calls stack height range is preserved.
* add subcontainer and top container size checks

Signed-off-by: Danno Ferrin <danno@numisight.com>
pull/7422/head
Danno Ferrin 4 months ago committed by GitHub
parent e3bc248990
commit a6a0271ae2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 5
      ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/PrettyPrintSubCommand.java
  2. 8
      ethereum/evmtool/src/test/resources/org/hyperledger/besu/evmtool/pretty-print/dangling-data.json
  3. 2
      evm/src/main/java/org/hyperledger/besu/evm/code/CodeV1Validation.java
  4. 7
      evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java
  5. 9
      evm/src/test/java/org/hyperledger/besu/evm/code/CodeV1Test.java
  6. 12
      evm/src/test/java/org/hyperledger/besu/evm/code/EOFLayoutTest.java

@ -110,6 +110,11 @@ public class PrettyPrintSubCommand implements Runnable {
if (validatedCode instanceof CodeInvalid codeInvalid) { if (validatedCode instanceof CodeInvalid codeInvalid) {
parentCommand.out.println("EOF code is invalid - " + codeInvalid.getInvalidReason()); parentCommand.out.println("EOF code is invalid - " + codeInvalid.getInvalidReason());
} }
if (layout.container().size() != container.size()) {
parentCommand.out.println(
"EOF code is invalid - dangling data after container - "
+ container.slice(layout.container().size()).toHexString());
}
} else { } else {
parentCommand.out.println("EOF layout is invalid - " + layout.invalidReason()); parentCommand.out.println("EOF layout is invalid - " + layout.invalidReason());
} }

@ -0,0 +1,8 @@
{
"cli": [
"pretty-print",
"ef00010100040200010001040000000080000000ff"
],
"stdin": "",
"stdout": "EOF layout is invalid - Dangling data after end of all sections\n"
}

@ -486,6 +486,8 @@ public class CodeV1Validation implements EOFValidator {
"Code that was not forward referenced in section 0x%x pc %d", "Code that was not forward referenced in section 0x%x pc %d",
codeSectionToValidate, currentPC); codeSectionToValidate, currentPC);
} }
currentMin = min(stack_min[currentPC], currentMin);
currentMax = max(stack_max[currentPC], currentMax);
if (stackInputs > currentMin) { if (stackInputs > currentMin) {
return format( return format(

@ -191,6 +191,7 @@ public record EOFLayout(
* strict and excess data is in the container * strict and excess data is in the container
* @return the eof layout * @return the eof layout
*/ */
@SuppressWarnings("ReferenceEquality")
public static EOFLayout parseEOF(final Bytes container, final boolean strictSize) { public static EOFLayout parseEOF(final Bytes container, final boolean strictSize) {
Queue<EOFParseStep> parseQueue = new ArrayDeque<>(); Queue<EOFParseStep> parseQueue = new ArrayDeque<>();
parseQueue.add(new EOFParseStep(container, strictSize, -1, null, null)); parseQueue.add(new EOFParseStep(container, strictSize, -1, null, null));
@ -213,8 +214,10 @@ public record EOFLayout(
+ " - " + " - "
+ parsedContainer.invalidReason); + parsedContainer.invalidReason);
} }
if (step.container.size() < parsedContainer.container.size()) { // This ReferenceEquality check is correct
return invalidLayout(container, parsedContainer.version, "excess data in subcontainer"); if ((strictSize || result != parsedContainer)
&& step.container.size() != parsedContainer.container.size()) {
return invalidLayout(container, parsedContainer.version, "subcontainer size mismatch");
} }
if (step.index >= 0) { if (step.index >= 0) {
step.parentSubcontainers[step.index] = parsedContainer; step.parentSubcontainers[step.index] = parsedContainer;

@ -30,7 +30,7 @@ import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource; import org.junit.jupiter.params.provider.ValueSource;
/** /**
* These tests focus on code only validations, which are checked within the code runs themselves. * These tests focus on code-only validations, which are checked within the code runs themselves.
* Tests that depend on the EOF container (such as CallF into other sections) are in EOFLayoutTest. * Tests that depend on the EOF container (such as CallF into other sections) are in EOFLayoutTest.
*/ */
class CodeV1Test { class CodeV1Test {
@ -520,7 +520,12 @@ class CodeV1Test {
"Stack underflow", "Stack underflow",
"Operation 0x50 requires stack of 1 but may only have 0 items", "Operation 0x50 requires stack of 1 but may only have 0 items",
0, 0,
List.of(List.of("50 00", 0, 0x80, 1)))); List.of(List.of("50 00", 0, 0x80, 1))),
Arguments.of(
"double rjumpi",
"Operation 0xF3 requires stack of 2 but may only have 1 items",
0,
List.of(List.of("5f 5f e10005 5f 5f e10000 f3", 0, 0x80, 1))));
} }
static Stream<Arguments> stackRJumpForward() { static Stream<Arguments> stackRJumpForward() {

@ -333,6 +333,18 @@ public class EOFLayoutTest {
null, null,
1 1
}, },
{
"EF00 01 010004 0200010001 0300010015 040000 00 00800000 00 (EF0001 010004 0200010001 040000 00 00800000 00ff)",
"dangling data in subcontainer",
"subcontainer size mismatch",
1
},
{
"EF00 01 010004 0200010001 0300010014 040000 00 00800000 00 (EF0001 010004 0200010001 040000 00 00800000 00ff)",
"dangling data in container",
"Dangling data after end of all sections",
1
},
}); });
} }

Loading…
Cancel
Save