diff --git a/accounts/abi/abi.go b/accounts/abi/abi.go index f75278c8b1..95bc66cce2 100644 --- a/accounts/abi/abi.go +++ b/accounts/abi/abi.go @@ -78,8 +78,12 @@ func (abi ABI) Pack(name string, args ...interface{}) ([]byte, error) { if err != nil { return nil, err } - // Pack up the method ID too if not a constructor and return - return append(method.ID, arguments...), nil + // Pack up the method ID too if not a constructor and return. The method ID + // is copied into a fresh slice instead of appended to directly: method.ID is + // derived from keccak256(sig)[:4], so it has length 4 but capacity 32, and a + // no-arg pack would otherwise return that backing array itself, aliasing the + // cached selector. + return append(append([]byte{}, method.ID...), arguments...), nil } func (abi ABI) getArguments(name string, data []byte) (Arguments, error) { diff --git a/accounts/abi/pack_selector_alias_test.go b/accounts/abi/pack_selector_alias_test.go new file mode 100644 index 0000000000..c7a5db13db --- /dev/null +++ b/accounts/abi/pack_selector_alias_test.go @@ -0,0 +1,68 @@ +// Copyright 2026 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package abi + +import ( + "bytes" + "strings" + "testing" +) + +// TestPackNoArgMethodDoesNotAliasSelector is a regression test for a bug where +// ABI.Pack of a zero-argument method returned a slice that aliased the cached +// method.ID. method.ID is keccak256(sig)[:4]: length 4 but capacity 32, so +// `append(method.ID, arguments...)` with no arguments returned the selector's +// own backing array. Mutating the returned calldata then corrupted the cached +// selector for every subsequent encode of that method. +func TestPackNoArgMethodDoesNotAliasSelector(t *testing.T) { + const def = `[{ "type" : "function", "name" : "balance", "stateMutability" : "view" }]` + abi, err := JSON(strings.NewReader(def)) + if err != nil { + t.Fatal(err) + } + // selector aliases the cached method.ID; want is an independent copy of the + // expected value, so it stays correct even if the bug corrupts the cache. + selector := abi.Methods["balance"].ID + want := bytes.Clone(selector) + + data1, err := abi.Pack("balance") + if err != nil { + t.Fatal(err) + } + // The returned calldata must be exactly the 4-byte selector... + if !bytes.Equal(data1, want) { + t.Fatalf("unexpected calldata: got %x, want %x", data1, want) + } + // ...but it must not share storage with the cached selector. + if len(data1) > 0 && len(selector) > 0 && &data1[0] == &selector[0] { + t.Fatal("Pack returned a slice aliasing the cached method.ID backing array") + } + + // Mutating the first result must not affect the selector or later packs. + data1[0] ^= 0xff + + data2, err := abi.Pack("balance") + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(data2, want) { + t.Fatalf("second pack corrupted: got %x, want %x", data2, want) + } + if !bytes.Equal(abi.Methods["balance"].ID, want) { + t.Fatalf("cached method.ID corrupted: got %x, want %x", abi.Methods["balance"].ID, want) + } +}