From a484bfacf0c991104c054cf297af62868e72766f Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Thu, 18 Jul 2024 13:25:32 +0200 Subject: [PATCH] core/vm: updated to v1.0.6, fix bugs --- cmd/eofdump/eofparser.go | 2 +- cmd/evm/internal/t8ntool/transition.go | 2 +- core/vm/eips.go | 2 +- core/vm/eof.go | 47 +++++++++++-- core/vm/evm.go | 2 +- core/vm/validate.go | 97 ++++++++++++++++++++------ core/vm/validate_test.go | 24 +++---- 7 files changed, 131 insertions(+), 45 deletions(-) diff --git a/cmd/eofdump/eofparser.go b/cmd/eofdump/eofparser.go index de5b4e500c..a50a1f0170 100644 --- a/cmd/eofdump/eofparser.go +++ b/cmd/eofdump/eofparser.go @@ -206,7 +206,7 @@ func parseAndValidate(s string, isInitCode bool) (*vm.Container, error) { if err := c.UnmarshalBinary(b, isInitCode); err != nil { return nil, err } - if err := c.ValidateCode(&jt); err != nil { + if err := c.ValidateCode(&jt, isInitCode); err != nil { return nil, err } return &c, nil diff --git a/cmd/evm/internal/t8ntool/transition.go b/cmd/evm/internal/t8ntool/transition.go index dfa27f3871..793539c1da 100644 --- a/cmd/evm/internal/t8ntool/transition.go +++ b/cmd/evm/internal/t8ntool/transition.go @@ -300,7 +300,7 @@ func applyEOFChecks(prestate *Prestate, chainConfig *params.ChainConfig) error { err = c.UnmarshalBinary(acc.Code, false) if err == nil { jt := vm.NewPragueEOFInstructionSetForTesting() - err = c.ValidateCode(&jt) + err = c.ValidateCode(&jt, false) } if err != nil { return NewError(ErrorConfig, fmt.Errorf("code at %s considered invalid: %v", addr, err)) diff --git a/core/vm/eips.go b/core/vm/eips.go index 59c405eb75..5e77580728 100644 --- a/core/vm/eips.go +++ b/core/vm/eips.go @@ -1005,7 +1005,7 @@ func opReturnContract(pc *uint64, interpreter *EVMInterpreter, scope *ScopeConte if err := c.UnmarshalBinary(deployedCode, true); err != nil { return nil, err } - if err := c.ValidateCode(interpreter.tableEOF); err != nil { + if err := c.ValidateCode(interpreter.tableEOF, true); err != nil { return nil, err } if len(c.Data) < c.DataSize { diff --git a/core/vm/eof.go b/core/vm/eof.go index 349ca4d5f7..48414716a1 100644 --- a/core/vm/eof.go +++ b/core/vm/eof.go @@ -23,6 +23,8 @@ import ( "errors" "fmt" "io" + + "github.com/ethereum/go-ethereum/params" ) const ( @@ -143,12 +145,19 @@ func (c *Container) MarshalBinary() []byte { // UnmarshalBinary decodes an EOF container. func (c *Container) UnmarshalBinary(b []byte, isInitcode bool) error { + return c.unmarshaSubContainer(b, isInitcode, true) +} + +func (c *Container) unmarshaSubContainer(b []byte, isInitcode bool, topLevel bool) error { if !hasEOFMagic(b) { return fmt.Errorf("%w: want %x", ErrInvalidMagic, eofMagic) } if len(b) < 14 { return io.ErrUnexpectedEOF } + if len(b) > params.MaxInitCodeSize { + return ErrMaxInitCodeSizeExceeded + } if !isEOFVersion1(b) { return fmt.Errorf("%w: have %d, want %d", ErrInvalidVersion, b[2], eof1Version) } @@ -282,7 +291,7 @@ func (c *Container) UnmarshalBinary(b []byte, isInitcode bool) error { } c := new(Container) end := min(idx+size, len(b)) - if err := c.UnmarshalBinary(b[idx:end], isInitcode); err != nil { + if err := c.unmarshaSubContainer(b[idx:end], isInitcode, false); err != nil { return fmt.Errorf("%w for section %d", err, i) } container = append(container, c) @@ -299,6 +308,9 @@ func (c *Container) UnmarshalBinary(b []byte, isInitcode bool) error { if !isInitcode { end = min(idx+dataSize, len(b)) } + if topLevel && len(b) != idx+dataSize { + return ErrTruncatedTopLevelContainer + } c.Data = b[idx:end] return nil @@ -306,8 +318,13 @@ func (c *Container) UnmarshalBinary(b []byte, isInitcode bool) error { // ValidateCode validates each code section of the container against the EOF v1 // rule set. -func (c *Container) ValidateCode(jt *JumpTable) error { +func (c *Container) ValidateCode(jt *JumpTable, isInitCode bool) error { + return c.validateSubContainer(jt, isInitCode, NotRefByEither) +} + +func (c *Container) validateSubContainer(jt *JumpTable, isInitCode bool, refBy int) error { visited := make(map[int]struct{}) + subContainerVisited := make(map[int]int) toVisit := []int{0} for len(toVisit) > 0 { // TODO check if this can be used as a DOS @@ -321,17 +338,31 @@ func (c *Container) ValidateCode(jt *JumpTable) error { code = c.Code[index] ) if _, ok := visited[index]; !ok { - v, err := validateCode(code, index, c, jt) + res, err := validateCode(code, index, c, jt, isInitCode) if err != nil { return err } visited[index] = struct{}{} // Mark all sections that can be visited from here. - for idx := range v { + for idx := range res.VisitedCode { if _, ok := visited[idx]; !ok { toVisit = append(toVisit, idx) } } + // Mark all subcontainer that can be visited from here. + for idx, reference := range res.VisitedSubContainers { + // Make sure subcontainers are only ever referenced by either EOFCreate or ReturnContract + if ref, ok := subContainerVisited[idx]; ok && ref != reference { + return errors.New("section referenced by both EOFCreate and ReturnContract") + } + subContainerVisited[idx] = reference + } + if refBy == RefByReturnContract && res.IsInitCode { + return ErrIncompatibleContainerKind + } + if refBy == RefByEOFCreate && res.IsRuntime { + return ErrIncompatibleContainerKind + } } toVisit = toVisit[1:] } @@ -339,8 +370,12 @@ func (c *Container) ValidateCode(jt *JumpTable) error { if len(visited) != len(c.Code) { return ErrUnreachableCode } - for _, container := range c.ContainerSections { - if err := container.ValidateCode(jt); err != nil { + for idx, container := range c.ContainerSections { + reference, ok := subContainerVisited[idx] + if !ok { + return ErrOrphanedSubcontainer + } + if err := container.validateSubContainer(jt, isInitCode, reference); err != nil { return err } } diff --git a/core/vm/evm.go b/core/vm/evm.go index 9060898190..25769731ca 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -493,7 +493,7 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64, if err := c.UnmarshalBinary(codeAndHash.code, isInitcodeEOF); err != nil { return nil, common.Address{}, gas, fmt.Errorf("%w: %v", ErrInvalidEOFInitcode, err) } - if err := c.ValidateCode(evm.interpreter.tableEOF); err != nil { + if err := c.ValidateCode(evm.interpreter.tableEOF, isInitcodeEOF); err != nil { return nil, common.Address{}, gas, fmt.Errorf("%w: %v", ErrInvalidEOFInitcode, err) } contract.Container = &c diff --git a/core/vm/validate.go b/core/vm/validate.go index 023baa4b75..7cb0b0600a 100644 --- a/core/vm/validate.go +++ b/core/vm/validate.go @@ -36,20 +36,41 @@ var ( ErrInvalidMaxStackHeight = errors.New("invalid max stack height") ErrInvalidCodeTermination = errors.New("invalid code termination") ErrEOFCreateWithTruncatedSection = errors.New("eofcreate with truncated section") + ErrOrphanedSubcontainer = errors.New("subcontainer not referenced at all") + ErrIncompatibleContainerKind = errors.New("incompatible container kind") + ErrStopAndReturnContract = errors.New("Stop/Return and Returncontract in the same code section") + ErrStopInInitCode = errors.New("initcode contains a RETURN or STOP opcode") + ErrTruncatedTopLevelContainer = errors.New("truncated top level container") ErrUnreachableCode = errors.New("unreachable code") ) +const ( + NotRefByEither = iota + RefByReturnContract + RefByEOFCreate +) + +type ValidationResult struct { + VisitedCode map[int]struct{} + VisitedSubContainers map[int]int + IsInitCode bool + IsRuntime bool +} + // validateCode validates the code parameter against the EOF v1 validity requirements. -func validateCode(code []byte, section int, container *Container, jt *JumpTable) (map[int]struct{}, error) { +func validateCode(code []byte, section int, container *Container, jt *JumpTable, isInitCode bool) (*ValidationResult, error) { var ( i = 0 // Tracks the number of actual instructions in the code (e.g. // non-immediate values). This is used at the end to determine // if each instruction is reachable. - count = 0 - op OpCode - analysis bitvec - visited = make(map[int]struct{}) + count = 0 + op OpCode + analysis bitvec + visitedCode = make(map[int]struct{}) + visitedSubcontainers = make(map[int]int) + hasReturnContract bool + hasStop bool ) // This loop visits every single instruction and verifies: // * if the instruction is valid for the given jump table. @@ -61,63 +82,88 @@ func validateCode(code []byte, section int, container *Container, jt *JumpTable) count++ op = OpCode(code[i]) if jt[op].undefined { - return visited, fmt.Errorf("%w: op %s, pos %d", ErrUndefinedInstruction, op, i) + return nil, fmt.Errorf("%w: op %s, pos %d", ErrUndefinedInstruction, op, i) } if size := jt[op].immediate; size != 0 { if len(code) <= i+size { - return visited, fmt.Errorf("%w: op %s, pos %d", ErrTruncatedImmediate, op, i) + return nil, fmt.Errorf("%w: op %s, pos %d", ErrTruncatedImmediate, op, i) } switch { case op == RJUMP || op == RJUMPI: if err := checkDest(code, &analysis, i+1, i+3, len(code)); err != nil { - return visited, err + return nil, err } case op == RJUMPV: max_size := int(code[i+1]) length := max_size + 1 if len(code) <= i+length { - return visited, fmt.Errorf("%w: jump table truncated, op %s, pos %d", ErrTruncatedImmediate, op, i) + return nil, fmt.Errorf("%w: jump table truncated, op %s, pos %d", ErrTruncatedImmediate, op, i) } offset := i + 2 for j := 0; j < length; j++ { if err := checkDest(code, &analysis, offset+j*2, offset+(length*2), len(code)); err != nil { - return visited, err + return nil, err } } i += 2 * max_size case op == CALLF: arg, _ := parseUint16(code[i+1:]) if arg >= len(container.Types) { - return visited, fmt.Errorf("%w: arg %d, last %d, pos %d", ErrInvalidSectionArgument, arg, len(container.Types), i) + return nil, fmt.Errorf("%w: arg %d, last %d, pos %d", ErrInvalidSectionArgument, arg, len(container.Types), i) } if container.Types[arg].Output == 0x80 { - return visited, fmt.Errorf("%w: section %v", ErrInvalidCallArgument, arg) + return nil, fmt.Errorf("%w: section %v", ErrInvalidCallArgument, arg) } - visited[arg] = struct{}{} + visitedCode[arg] = struct{}{} case op == JUMPF: arg, _ := parseUint16(code[i+1:]) if arg >= len(container.Types) { - return visited, fmt.Errorf("%w: arg %d, last %d, pos %d", ErrInvalidSectionArgument, arg, len(container.Types), i) + return nil, fmt.Errorf("%w: arg %d, last %d, pos %d", ErrInvalidSectionArgument, arg, len(container.Types), i) } - visited[arg] = struct{}{} + visitedCode[arg] = struct{}{} case op == DATALOADN: arg, _ := parseUint16(code[i+1:]) if arg+32 > len(container.Data) { - return visited, fmt.Errorf("%w: arg %d, last %d, pos %d", ErrInvalidDataloadNArgument, arg, len(container.Data), i) + return nil, fmt.Errorf("%w: arg %d, last %d, pos %d", ErrInvalidDataloadNArgument, arg, len(container.Data), i) } case op == RETURNCONTRACT: arg := int(code[i+1]) if arg >= len(container.ContainerSections) { - return visited, fmt.Errorf("%w: arg %d, last %d, pos %d", ErrUnreachableCode, arg, len(container.ContainerSections), i) + return nil, fmt.Errorf("%w: arg %d, last %d, pos %d", ErrUnreachableCode, arg, len(container.ContainerSections), i) } + // We need to store per subcontainer how it was referenced + if v, ok := visitedSubcontainers[arg]; ok && v != RefByReturnContract { + return nil, fmt.Errorf("section already referenced, arg :%d", arg) + } + if hasStop { + return nil, ErrStopAndReturnContract + } + hasReturnContract = true + visitedSubcontainers[arg] = RefByReturnContract case op == EOFCREATE: arg := int(code[i+1]) if arg >= len(container.ContainerSections) { - return visited, fmt.Errorf("%w: arg %d, last %d, pos %d", ErrUnreachableCode, arg, len(container.ContainerSections), i) + return nil, fmt.Errorf("%w: arg %d, last %d, pos %d", ErrUnreachableCode, arg, len(container.ContainerSections), i) } if ct := container.ContainerSections[arg]; len(ct.Data) != ct.DataSize { - return visited, fmt.Errorf("%w: container %d, have %d, claimed %d, pos %d", ErrEOFCreateWithTruncatedSection, arg, len(ct.Data), ct.DataSize, i) + return nil, fmt.Errorf("%w: container %d, have %d, claimed %d, pos %d", ErrEOFCreateWithTruncatedSection, arg, len(ct.Data), ct.DataSize, i) } + if _, ok := visitedSubcontainers[arg]; ok { + return nil, fmt.Errorf("section already referenced, arg :%d", arg) + } + // We need to store per subcontainer how it was referenced + if v, ok := visitedSubcontainers[arg]; ok && v != RefByEOFCreate { + return nil, fmt.Errorf("section already referenced, arg :%d", arg) + } + visitedSubcontainers[arg] = RefByEOFCreate + case op == STOP || op == RETURN: + if isInitCode { + return nil, ErrStopInInitCode + } + if hasReturnContract { + return nil, ErrStopAndReturnContract + } + hasStop = true } i += size } @@ -126,16 +172,21 @@ func validateCode(code []byte, section int, container *Container, jt *JumpTable) // Code sections may not "fall through" and require proper termination. // Therefore, the last instruction must be considered terminal or RJUMP. if !jt[op].terminal && op != RJUMP { - return visited, fmt.Errorf("%w: end with %s, pos %d", ErrInvalidCodeTermination, op, i) + return nil, fmt.Errorf("%w: end with %s, pos %d", ErrInvalidCodeTermination, op, i) } if paths, err := validateControlFlow2(code, section, container.Types, jt); err != nil { - return visited, err + return nil, err } else if paths != count { fmt.Printf("Paths: %v Count: %v\n", paths, count) // TODO(matt): return actual position of unreachable code - return visited, ErrUnreachableCode + return nil, ErrUnreachableCode } - return visited, nil + return &ValidationResult{ + VisitedCode: visitedCode, + VisitedSubContainers: visitedSubcontainers, + IsInitCode: hasReturnContract, + IsRuntime: hasStop, + }, nil } // checkDest parses a relative offset at code[0:2] and checks if it is a valid jump destination. diff --git a/core/vm/validate_test.go b/core/vm/validate_test.go index 4d80117a65..ee444a76fb 100644 --- a/core/vm/validate_test.go +++ b/core/vm/validate_test.go @@ -250,7 +250,7 @@ func TestValidateCode(t *testing.T) { Data: make([]byte, 0), ContainerSections: make([]*Container, 0), } - _, err := validateCode(test.code, test.section, container, &pragueEOFInstructionSet) + _, err := validateCode(test.code, test.section, container, &pragueEOFInstructionSet, true) if !errors.Is(err, test.err) { t.Errorf("test %d (%s): unexpected error (want: %v, got: %v)", i, common.Bytes2Hex(test.code), test.err, err) } @@ -274,7 +274,7 @@ func BenchmarkRJUMPI(b *testing.B) { } b.ResetTimer() for i := 0; i < b.N; i++ { - _, err := validateCode(code, 0, container, &pragueEOFInstructionSet) + _, err := validateCode(code, 0, container, &pragueEOFInstructionSet, true) if err != nil { b.Fatal(err) } @@ -304,7 +304,7 @@ func BenchmarkRJUMPV(b *testing.B) { } b.ResetTimer() for i := 0; i < b.N; i++ { - _, err := validateCode(code, 0, container, &pragueEOFInstructionSet) + _, err := validateCode(code, 0, container, &pragueEOFInstructionSet, true) if err != nil { b.Fatal(err) } @@ -345,10 +345,10 @@ func BenchmarkEOFValidation(b *testing.B) { var container2 Container b.ResetTimer() for i := 0; i < b.N; i++ { - if err := container2.UnmarshalBinary(bin); err != nil { + if err := container2.UnmarshalBinary(bin, true); err != nil { b.Fatal(err) } - if err := container2.ValidateCode(&pragueEOFInstructionSet); err != nil { + if err := container2.ValidateCode(&pragueEOFInstructionSet, true); err != nil { b.Fatal(err) } } @@ -395,10 +395,10 @@ func BenchmarkEOFValidation2(b *testing.B) { var container2 Container b.ResetTimer() for i := 0; i < b.N; i++ { - if err := container2.UnmarshalBinary(bin); err != nil { + if err := container2.UnmarshalBinary(bin, true); err != nil { b.Fatal(err) } - if err := container2.ValidateCode(&pragueEOFInstructionSet); err != nil { + if err := container2.ValidateCode(&pragueEOFInstructionSet, true); err != nil { b.Fatal(err) } } @@ -444,10 +444,10 @@ func BenchmarkEOFValidation3(b *testing.B) { for i := 0; i < b.N; i++ { for k := 0; k < 40; k++ { var container2 Container - if err := container2.UnmarshalBinary(bin); err != nil { + if err := container2.UnmarshalBinary(bin, true); err != nil { b.Fatal(err) } - if err := container2.ValidateCode(&pragueEOFInstructionSet); err != nil { + if err := container2.ValidateCode(&pragueEOFInstructionSet, true); err != nil { b.Fatal(err) } } @@ -473,7 +473,7 @@ func BenchmarkRJUMPI_2(b *testing.B) { } b.ResetTimer() for i := 0; i < b.N; i++ { - _, err := validateCode(code, 0, container, &pragueEOFInstructionSet) + _, err := validateCode(code, 0, container, &pragueEOFInstructionSet, true) if err != nil { b.Fatal(err) } @@ -483,7 +483,7 @@ func BenchmarkRJUMPI_2(b *testing.B) { func FuzzUnmarshalBinary(f *testing.F) { f.Fuzz(func(_ *testing.T, input []byte) { var container Container - container.UnmarshalBinary(input) + container.UnmarshalBinary(input, true) }) } @@ -491,6 +491,6 @@ func FuzzValidate(f *testing.F) { f.Fuzz(func(_ *testing.T, code []byte, maxStack uint16) { var container Container container.Types = append(container.Types, &FunctionMetadata{Input: 0, Output: 0x80, MaxStackHeight: maxStack}) - validateCode(code, 0, &container, &pragueEOFInstructionSet) + validateCode(code, 0, &container, &pragueEOFInstructionSet, true) }) }