From 3026268f1542da4d329cf016e81b6455d82ddc61 Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Thu, 20 Jun 2024 10:20:40 -0600 Subject: [PATCH] Check for EOFCreate subcontainer rules (#7232) Check and test for the unused container rule, and only returncontract targets can have truncated data rule. Also test the other subcontainer rules in unit tests. Signed-off-by: Danno Ferrin --- .../ethereum/eof/EOFReferenceTestTools.java | 18 + .../besu/evm/code/CodeV1Validation.java | 12 + .../hyperledger/besu/evm/code/EOFLayout.java | 10 + .../besu/evm/code/CodeFactoryTest.java | 438 +++++++++++++++++- .../besu/evm/code/EOFLayoutTest.java | 16 +- 5 files changed, 480 insertions(+), 14 deletions(-) diff --git a/ethereum/referencetests/src/reference-test/java/org/hyperledger/besu/ethereum/eof/EOFReferenceTestTools.java b/ethereum/referencetests/src/reference-test/java/org/hyperledger/besu/ethereum/eof/EOFReferenceTestTools.java index cf7ce66b076..22bf3839010 100644 --- a/ethereum/referencetests/src/reference-test/java/org/hyperledger/besu/ethereum/eof/EOFReferenceTestTools.java +++ b/ethereum/referencetests/src/reference-test/java/org/hyperledger/besu/ethereum/eof/EOFReferenceTestTools.java @@ -72,6 +72,24 @@ public class EOFReferenceTestTools { // TXCREATE still in tests, but has been removed params.ignore("EOF1_undefined_opcodes_186"); + + // embedded containers rules changed + params.ignore("EOF1_embedded_container"); + + // truncated data is only allowed in embedded containers + params.ignore("ori/validInvalid-Prague\\[validInvalid_48\\]"); + params.ignore("efExample/validInvalid-Prague\\[validInvalid_1\\]"); + params.ignore("efValidation/EOF1_truncated_section-Prague\\[EOF1_truncated_section_3\\]"); + params.ignore("efValidation/EOF1_truncated_section-Prague\\[EOF1_truncated_section_4\\]"); + params.ignore("EIP3540/validInvalid-Prague\\[validInvalid_2\\]"); + params.ignore("EIP3540/validInvalid-Prague\\[validInvalid_3\\]"); + + // Orphan containers are no longer allowed + params.ignore("efValidation/EOF1_returncontract_valid-Prague\\[EOF1_returncontract_valid_1\\]"); + params.ignore("efValidation/EOF1_returncontract_valid-Prague\\[EOF1_returncontract_valid_2\\]"); + params.ignore("efValidation/EOF1_eofcreate_valid-Prague\\[EOF1_eofcreate_valid_1\\]"); + params.ignore("efValidation/EOF1_eofcreate_valid-Prague\\[EOF1_eofcreate_valid_2\\]"); + params.ignore("efValidation/EOF1_section_order-Prague\\[EOF1_section_order_6\\]"); } private EOFReferenceTestTools() { diff --git a/evm/src/main/java/org/hyperledger/besu/evm/code/CodeV1Validation.java b/evm/src/main/java/org/hyperledger/besu/evm/code/CodeV1Validation.java index b52f1dfebbd..c940a7d930b 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/code/CodeV1Validation.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/code/CodeV1Validation.java @@ -67,6 +67,8 @@ private CodeV1Validation() { * @param layout The parsed EOFLayout of the code * @return either null, indicating no error, or a String describing the validation error. */ + @SuppressWarnings( + "ReferenceEquality") // comparison `container != layout` is deliberate and correct public static String validate(final EOFLayout layout) { Queue workList = new ArrayDeque<>(layout.getSubcontainerCount()); workList.add(layout); @@ -74,6 +76,16 @@ public static String validate(final EOFLayout layout) { while (!workList.isEmpty()) { EOFLayout container = workList.poll(); workList.addAll(List.of(container.subContainers())); + if (container != layout && container.containerMode().get() == null) { + return "Unreferenced container #" + layout.indexOfSubcontainer(container); + } + if (container.containerMode().get() != RUNTIME + && container.data().size() != container.dataLength()) { + return "Incomplete data section " + + (container == layout + ? " at root" + : " in container #" + layout.indexOfSubcontainer(container)); + } final String codeValidationError = CodeV1Validation.validateCode(container); if (codeValidationError != null) { diff --git a/evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java b/evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java index 95ad7f95dd2..1cce49b4531 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java @@ -405,6 +405,16 @@ public EOFLayout getSubcontainer(final int i) { return subContainers[i]; } + /** + * Finds the first instance of the subcontainer in the list of container, or -1 if not present + * + * @param container the container to search for + * @return the index of the container, or -1 if not found. + */ + public int indexOfSubcontainer(final EOFLayout container) { + return Arrays.asList(subContainers).indexOf(container); + } + /** * Is valid. * diff --git a/evm/src/test/java/org/hyperledger/besu/evm/code/CodeFactoryTest.java b/evm/src/test/java/org/hyperledger/besu/evm/code/CodeFactoryTest.java index d3335d077cf..75ec9eb01bf 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/code/CodeFactoryTest.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/code/CodeFactoryTest.java @@ -15,10 +15,10 @@ package org.hyperledger.besu.evm.code; import static org.assertj.core.api.Assertions.assertThat; +import static org.hyperledger.besu.evm.EOFTestConstants.bytesFromPrettyPrint; import org.hyperledger.besu.evm.Code; -import org.apache.tuweni.bytes.Bytes; import org.junit.jupiter.api.Test; class CodeFactoryTest { @@ -178,13 +178,445 @@ void invalidCodeUnknownSectionId3() { invalidCode("0xEF0001010002030004006000AABBCCDD"); } + @Test + void invalidDataTruncated() { + invalidCode("EF0001 010004 0200010001 040003 00 00800000 FE BEEF", "Incomplete data section"); + } + + @Test + void validComboEOFCreateReturnContract() { + validCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0011 # Code section 0 , 17 bytes + 030001 # Total subcontainers ( 1 ) + 0032 # Sub container 0, 50 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0004 # max stack: 4 + # Code section 0 - in=0 out=non-returning height=4 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + 6000 # [4] PUSH1(0) + 6000 # [6] PUSH1(0) + ec00 # [8] EOFCREATE(0) + 612015 # [10] PUSH2(0x2015) + 6001 # [13] PUSH1(1) + 55 # [15] SSTORE + 00 # [16] STOP + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0006 # Code section 0 , 6 bytes + 030001 # Total subcontainers ( 1 ) + 0014 # Sub container 0, 20 byte + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + ee00 # [4] RETURNCONTRACT(0) + # Subcontainer 0.0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0001 # Code section 0 , 1 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0000 # max stack: 0 + # Code section 0 - in=0 out=non-returning height=0 + 00 # [0] STOP + # Data section (empty) + # Subcontainer 0.0 ends + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """); + } + + @Test + void validComboReturnContactStop() { + validCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 000c # Code section 0 , 12 bytes + 030001 # Total subcontainers ( 1 ) + 0014 # Sub container 0, 20 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 612015 # [0] PUSH2(0x2015) + 6001 # [3] PUSH1(1) + 55 # [5] SSTORE + 6000 # [6] PUSH1(0) + 6000 # [8] PUSH1(0) + ee00 # [10] RETURNCONTRACT(0) + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0001 # Code section 0 , 1 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0000 # max stack: 0 + # Code section 0 - in=0 out=non-returning height=0 + 00 # [0] STOP + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """); + } + + @Test + void validComboReturnContactReturn() { + validCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 000c # Code section 0 , 12 bytes + 030001 # Total subcontainers ( 1 ) + 0018 # Sub container 0, 24 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 612015 # [0] PUSH2(0x2015) + 6001 # [3] PUSH1(1) + 55 # [5] SSTORE + 6000 # [6] PUSH1(0) + 6000 # [8] PUSH1(0) + ee00 # [10] RETURNCONTRACT(0) + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0005 # Code section 0 , 5 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + f3 # [4] RETURN + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """); + } + + @Test + void validComboEOFCreateRevert() { + validCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0011 # Code section 0 , 17 bytes + 030001 # Total subcontainers ( 1 ) + 0018 # Sub container 0, 24 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0004 # max stack: 4 + # Code section 0 - in=0 out=non-returning height=4 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + 6000 # [4] PUSH1(0) + 6000 # [6] PUSH1(0) + ec00 # [8] EOFCREATE(0) + 612015 # [10] PUSH2(0x2015) + 6001 # [13] PUSH1(1) + 55 # [15] SSTORE + 00 # [16] STOP + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0005 # Code section 0 , 5 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + fd # [4] REVERT + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """); + } + + @Test + void validComboReturncontractRevert() { + validCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 000c # Code section 0 , 12 bytes + 030001 # Total subcontainers ( 1 ) + 0018 # Sub container 0, 24 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 612015 # [0] PUSH2(0x2015) + 6001 # [3] PUSH1(1) + 55 # [5] SSTORE + 6000 # [6] PUSH1(0) + 6000 # [8] PUSH1(0) + ee00 # [10] RETURNCONTRACT(0) + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0005 # Code section 0 , 5 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + fd # [4] REVERT + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """); + } + + @Test + void invalidComboEOFCreateStop() { + invalidCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0011 # Code section 0 , 17 bytes + 030001 # Total subcontainers ( 1 ) + 0014 # Sub container 0, 20 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0004 # max stack: 4 + # Code section 0 - in=0 out=non-returning height=4 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + 6000 # [4] PUSH1(0) + 6000 # [6] PUSH1(0) + ec00 # [8] EOFCREATE(0) + 612015 # [10] PUSH2(0x2015) + 6001 # [13] PUSH1(1) + 55 # [15] SSTORE + 00 # [16] STOP + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0001 # Code section 0 , 1 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0000 # max stack: 0 + # Code section 0 - in=0 out=non-returning height=0 + 00 # [0] STOP + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """, + "STOP is only a valid opcode in containers used for runtime operations."); + } + + @Test + void invalidComboEOFCretateReturn() { + invalidCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0011 # Code section 0 , 17 bytes + 030001 # Total subcontainers ( 1 ) + 0018 # Sub container 0, 24 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0004 # max stack: 4 + # Code section 0 - in=0 out=non-returning height=4 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + 6000 # [4] PUSH1(0) + 6000 # [6] PUSH1(0) + ec00 # [8] EOFCREATE(0) + 612015 # [10] PUSH2(0x2015) + 6001 # [13] PUSH1(1) + 55 # [15] SSTORE + 00 # [16] STOP + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0005 # Code section 0 , 5 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + f3 # [4] RETURN + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """, + "RETURN is only a valid opcode in containers used for runtime operations."); + } + + @Test + void invalidReturncontractReturncontract() { + invalidCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 000c # Code section 0 , 12 bytes + 030001 # Total subcontainers ( 1 ) + 0032 # Sub container 0, 50 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 612015 # [0] PUSH2(0x2015) + 6001 # [3] PUSH1(1) + 55 # [5] SSTORE + 6000 # [6] PUSH1(0) + 6000 # [8] PUSH1(0) + ee00 # [10] RETURNCONTRACT(0) + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0006 # Code section 0 , 6 bytes + 030001 # Total subcontainers ( 1 ) + 0014 # Sub container 0, 20 byte + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + ee00 # [4] RETURNCONTRACT(0) + # Subcontainer 0.0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0001 # Code section 0 , 1 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0000 # max stack: 0 + # Code section 0 - in=0 out=non-returning height=0 + 00 # [0] STOP + # Data section (empty) + # Subcontainer 0.0 ends + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """, + "RETURNCONTRACT is only a valid opcode in containers used for initcode"); + } + + // // valid subcontainer references + // // invalid subcontainer references + // + // { + // "EF0001 010004 0200010001 040003 00 00800000 FE BEEF", + // "Incomplete data section", + // "Incomplete data section", + // 1 + // }, + // + + private static void validCode(final String str) { + Code code = CodeFactory.createCode(bytesFromPrettyPrint(str), 1); + assertThat(code.isValid()).isTrue(); + } + + private static void invalidCode(final String str, final String error) { + Code code = CodeFactory.createCode(bytesFromPrettyPrint(str), 1); + assertThat(code.isValid()).isFalse(); + assertThat(((CodeInvalid) code).getInvalidReason()).contains(error); + } + private static void invalidCode(final String str) { - Code code = CodeFactory.createCode(Bytes.fromHexString(str), 1); + Code code = CodeFactory.createCode(bytesFromPrettyPrint(str), 1); assertThat(code.isValid()).isFalse(); } private static void invalidCode(final String str, final boolean legacy) { - Code code = CodeFactory.createCode(Bytes.fromHexString(str), 1, legacy, false); + Code code = CodeFactory.createCode(bytesFromPrettyPrint(str), 1, legacy, false); assertThat(code.isValid()).isFalse(); } } diff --git a/evm/src/test/java/org/hyperledger/besu/evm/code/EOFLayoutTest.java b/evm/src/test/java/org/hyperledger/besu/evm/code/EOFLayoutTest.java index 94f1d9598e0..5324d7adf05 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/code/EOFLayoutTest.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/code/EOFLayoutTest.java @@ -143,16 +143,10 @@ public static Collection containersWithFormatErrors() { }, { "EF0001 010004 0200010001 040003 00 00800000 FE DEADBEEF", - "Incomplete data section", + "Excess data section", "Dangling data after end of all sections", 1 }, - { - "EF0001 010004 0200010001 040003 00 00800000 FE BEEF", - "Incomplete data section", - "Incomplete data section", - 1 - }, { "EF0001 0200010001 040001 00 FE DA", "type section missing", @@ -343,9 +337,9 @@ public static Collection subContainers() { @ParameterizedTest(name = "{1}") @MethodSource({ - // "correctContainers", - // "containersWithFormatErrors", - // "typeSectionTests", + "correctContainers", + "containersWithFormatErrors", + "typeSectionTests", "subContainers" }) void test( @@ -354,7 +348,7 @@ void test( final String failureReason, final int expectedVersion) { final Bytes container = Bytes.fromHexString(containerString.replaceAll("[^a-fxA-F0-9]", "")); - final EOFLayout layout = EOFLayout.parseEOF(container); + final EOFLayout layout = EOFLayout.parseEOF(container, true); assertThat(layout.version()).isEqualTo(expectedVersion); assertThat(layout.invalidReason()).isEqualTo(failureReason);