From 5500cbf0e420a6d643835ec05f35abb170e3e443 Mon Sep 17 00:00:00 2001 From: Jes Cok Date: Thu, 25 Sep 2025 04:41:12 +0000 Subject: [PATCH 1/4] debug/elf: prevent offset overflow When applying relocations, a malformed ELF file can provide an offset that, when added to the relocation size, overflows. This wrapped-around value could then incorrectly pass the bounds check, leading to a panic when the slice is accessed with the original large offset. This change eliminates the manual bounds and overflow checks and writes a relocation to slice by calling putUint. The putUint helper function centralizes the logic for validating slice access, correctly handling both out-of-bounds and integer overflow conditions. This simplifies the relocation code and improves robustness when parsing malformed ELF files. Fixes #75516 Change-Id: I00d806bf5501a9bf70200585ba4fd0475d7b2ddc GitHub-Last-Rev: 49144311d31fecc63cb81b6c31bf9a206acb0596 GitHub-Pull-Request: golang/go#75522 Reviewed-on: https://go-review.googlesource.com/c/go/+/705075 Reviewed-by: Florian Lehner LUCI-TryBot-Result: Go LUCI Reviewed-by: Junyang Shao Auto-Submit: Ian Lance Taylor Reviewed-by: Michael Knyszek Reviewed-by: Ian Lance Taylor Commit-Queue: Ian Lance Taylor --- src/debug/elf/file.go | 160 +++++++++++++++--------------------------- 1 file changed, 57 insertions(+), 103 deletions(-) diff --git a/src/debug/elf/file.go b/src/debug/elf/file.go index 50452b5bef45f4..1d56a06c3fb221 100644 --- a/src/debug/elf/file.go +++ b/src/debug/elf/file.go @@ -25,6 +25,7 @@ import ( "internal/saferio" "internal/zstd" "io" + "math" "os" "strings" "unsafe" @@ -830,17 +831,9 @@ func (f *File) applyRelocationsAMD64(dst []byte, rels []byte) error { switch t { case R_X86_64_64: - if rela.Off+8 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val64 := sym.Value + uint64(rela.Addend) - f.ByteOrder.PutUint64(dst[rela.Off:rela.Off+8], val64) + putUint(f.ByteOrder, dst, rela.Off, 8, sym.Value, rela.Addend, false) case R_X86_64_32: - if rela.Off+4 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val32 := uint32(sym.Value) + uint32(rela.Addend) - f.ByteOrder.PutUint32(dst[rela.Off:rela.Off+4], val32) + putUint(f.ByteOrder, dst, rela.Off, 4, sym.Value, rela.Addend, false) } } @@ -872,12 +865,7 @@ func (f *File) applyRelocations386(dst []byte, rels []byte) error { sym := &symbols[symNo-1] if t == R_386_32 { - if rel.Off+4 >= uint32(len(dst)) { - continue - } - val := f.ByteOrder.Uint32(dst[rel.Off : rel.Off+4]) - val += uint32(sym.Value) - f.ByteOrder.PutUint32(dst[rel.Off:rel.Off+4], val) + putUint(f.ByteOrder, dst, uint64(rel.Off), 4, sym.Value, 0, true) } } @@ -910,12 +898,7 @@ func (f *File) applyRelocationsARM(dst []byte, rels []byte) error { switch t { case R_ARM_ABS32: - if rel.Off+4 >= uint32(len(dst)) { - continue - } - val := f.ByteOrder.Uint32(dst[rel.Off : rel.Off+4]) - val += uint32(sym.Value) - f.ByteOrder.PutUint32(dst[rel.Off:rel.Off+4], val) + putUint(f.ByteOrder, dst, uint64(rel.Off), 4, sym.Value, 0, true) } } @@ -955,17 +938,9 @@ func (f *File) applyRelocationsARM64(dst []byte, rels []byte) error { switch t { case R_AARCH64_ABS64: - if rela.Off+8 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val64 := sym.Value + uint64(rela.Addend) - f.ByteOrder.PutUint64(dst[rela.Off:rela.Off+8], val64) + putUint(f.ByteOrder, dst, rela.Off, 8, sym.Value, rela.Addend, false) case R_AARCH64_ABS32: - if rela.Off+4 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val32 := uint32(sym.Value) + uint32(rela.Addend) - f.ByteOrder.PutUint32(dst[rela.Off:rela.Off+4], val32) + putUint(f.ByteOrder, dst, rela.Off, 4, sym.Value, rela.Addend, false) } } @@ -1001,11 +976,7 @@ func (f *File) applyRelocationsPPC(dst []byte, rels []byte) error { switch t { case R_PPC_ADDR32: - if rela.Off+4 >= uint32(len(dst)) || rela.Addend < 0 { - continue - } - val32 := uint32(sym.Value) + uint32(rela.Addend) - f.ByteOrder.PutUint32(dst[rela.Off:rela.Off+4], val32) + putUint(f.ByteOrder, dst, uint64(rela.Off), 4, sym.Value, 0, false) } } @@ -1041,17 +1012,9 @@ func (f *File) applyRelocationsPPC64(dst []byte, rels []byte) error { switch t { case R_PPC64_ADDR64: - if rela.Off+8 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val64 := sym.Value + uint64(rela.Addend) - f.ByteOrder.PutUint64(dst[rela.Off:rela.Off+8], val64) + putUint(f.ByteOrder, dst, rela.Off, 8, sym.Value, rela.Addend, false) case R_PPC64_ADDR32: - if rela.Off+4 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val32 := uint32(sym.Value) + uint32(rela.Addend) - f.ByteOrder.PutUint32(dst[rela.Off:rela.Off+4], val32) + putUint(f.ByteOrder, dst, rela.Off, 4, sym.Value, rela.Addend, false) } } @@ -1084,12 +1047,7 @@ func (f *File) applyRelocationsMIPS(dst []byte, rels []byte) error { switch t { case R_MIPS_32: - if rel.Off+4 >= uint32(len(dst)) { - continue - } - val := f.ByteOrder.Uint32(dst[rel.Off : rel.Off+4]) - val += uint32(sym.Value) - f.ByteOrder.PutUint32(dst[rel.Off:rel.Off+4], val) + putUint(f.ByteOrder, dst, uint64(rel.Off), 4, sym.Value, 0, true) } } @@ -1132,17 +1090,9 @@ func (f *File) applyRelocationsMIPS64(dst []byte, rels []byte) error { switch t { case R_MIPS_64: - if rela.Off+8 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val64 := sym.Value + uint64(rela.Addend) - f.ByteOrder.PutUint64(dst[rela.Off:rela.Off+8], val64) + putUint(f.ByteOrder, dst, rela.Off, 8, sym.Value, rela.Addend, false) case R_MIPS_32: - if rela.Off+4 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val32 := uint32(sym.Value) + uint32(rela.Addend) - f.ByteOrder.PutUint32(dst[rela.Off:rela.Off+4], val32) + putUint(f.ByteOrder, dst, rela.Off, 4, sym.Value, rela.Addend, false) } } @@ -1180,17 +1130,9 @@ func (f *File) applyRelocationsLOONG64(dst []byte, rels []byte) error { switch t { case R_LARCH_64: - if rela.Off+8 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val64 := sym.Value + uint64(rela.Addend) - f.ByteOrder.PutUint64(dst[rela.Off:rela.Off+8], val64) + putUint(f.ByteOrder, dst, rela.Off, 8, sym.Value, rela.Addend, false) case R_LARCH_32: - if rela.Off+4 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val32 := uint32(sym.Value) + uint32(rela.Addend) - f.ByteOrder.PutUint32(dst[rela.Off:rela.Off+4], val32) + putUint(f.ByteOrder, dst, rela.Off, 4, sym.Value, rela.Addend, false) } } @@ -1226,17 +1168,9 @@ func (f *File) applyRelocationsRISCV64(dst []byte, rels []byte) error { switch t { case R_RISCV_64: - if rela.Off+8 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val64 := sym.Value + uint64(rela.Addend) - f.ByteOrder.PutUint64(dst[rela.Off:rela.Off+8], val64) + putUint(f.ByteOrder, dst, rela.Off, 8, sym.Value, rela.Addend, false) case R_RISCV_32: - if rela.Off+4 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val32 := uint32(sym.Value) + uint32(rela.Addend) - f.ByteOrder.PutUint32(dst[rela.Off:rela.Off+4], val32) + putUint(f.ByteOrder, dst, rela.Off, 4, sym.Value, rela.Addend, false) } } @@ -1272,17 +1206,9 @@ func (f *File) applyRelocationss390x(dst []byte, rels []byte) error { switch t { case R_390_64: - if rela.Off+8 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val64 := sym.Value + uint64(rela.Addend) - f.ByteOrder.PutUint64(dst[rela.Off:rela.Off+8], val64) + putUint(f.ByteOrder, dst, rela.Off, 8, sym.Value, rela.Addend, false) case R_390_32: - if rela.Off+4 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val32 := uint32(sym.Value) + uint32(rela.Addend) - f.ByteOrder.PutUint32(dst[rela.Off:rela.Off+4], val32) + putUint(f.ByteOrder, dst, rela.Off, 4, sym.Value, rela.Addend, false) } } @@ -1318,17 +1244,10 @@ func (f *File) applyRelocationsSPARC64(dst []byte, rels []byte) error { switch t { case R_SPARC_64, R_SPARC_UA64: - if rela.Off+8 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val64 := sym.Value + uint64(rela.Addend) - f.ByteOrder.PutUint64(dst[rela.Off:rela.Off+8], val64) + putUint(f.ByteOrder, dst, rela.Off, 8, sym.Value, rela.Addend, false) + case R_SPARC_32, R_SPARC_UA32: - if rela.Off+4 >= uint64(len(dst)) || rela.Addend < 0 { - continue - } - val32 := uint32(sym.Value) + uint32(rela.Addend) - f.ByteOrder.PutUint32(dst[rela.Off:rela.Off+4], val32) + putUint(f.ByteOrder, dst, rela.Off, 4, sym.Value, rela.Addend, false) } } @@ -1903,3 +1822,38 @@ type nobitsSectionReader struct{} func (*nobitsSectionReader) ReadAt(p []byte, off int64) (n int, err error) { return 0, errors.New("unexpected read from SHT_NOBITS section") } + +// putUint writes a relocation to slice +// at offset start of length length (4 or 8 bytes), +// adding sym+addend to the existing value if readUint is true, +// or just writing sym+addend if readUint is false. +// If the write would extend beyond the end of slice, putUint does nothing. +// If the addend is negative, putUint does nothing. +// If the addition would overflow, putUint does nothing. +func putUint(byteOrder binary.ByteOrder, slice []byte, start, length, sym uint64, addend int64, readUint bool) { + if start+length > uint64(len(slice)) || math.MaxUint64-start < length { + return + } + if addend < 0 { + return + } + + s := slice[start : start+length] + + switch length { + case 4: + ae := uint32(addend) + if readUint { + ae += byteOrder.Uint32(s) + } + byteOrder.PutUint32(s, uint32(sym)+ae) + case 8: + ae := uint64(addend) + if readUint { + ae += byteOrder.Uint64(s) + } + byteOrder.PutUint64(s, sym+ae) + default: + panic("can't happen") + } +} From 8021c4459b4b02f96846a6a678783204881cba5e Mon Sep 17 00:00:00 2001 From: Robbie McMichael Date: Mon, 29 Sep 2025 14:58:22 +0800 Subject: [PATCH 2/4] cmd/gofmt: change -d to exit 1 if diffs exist When using the -d flag, set the exit code to 1 if there is a diff. Fixes #46289 --- src/cmd/gofmt/gofmt.go | 12 ++++++++++-- src/cmd/gofmt/gofmt_test.go | 23 +++++++++++++++++++++-- src/cmd/gofmt/testdata/exitcode0 | 4 ++++ src/cmd/gofmt/testdata/exitcode1 | 4 ++++ 4 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 src/cmd/gofmt/testdata/exitcode0 create mode 100644 src/cmd/gofmt/testdata/exitcode1 diff --git a/src/cmd/gofmt/gofmt.go b/src/cmd/gofmt/gofmt.go index bbb8b4fd15c2f7..ad6ad636524479 100644 --- a/src/cmd/gofmt/gofmt.go +++ b/src/cmd/gofmt/gofmt.go @@ -41,6 +41,9 @@ var ( // debugging cpuprofile = flag.String("cpuprofile", "", "write cpu profile to this file") + + // errors + errFormattingDiffers = fmt.Errorf("formatting differs from gofmt's") ) // Keep these in sync with go/format/format.go. @@ -218,8 +221,12 @@ func (r *reporter) Report(err error) { panic("Report with nil error") } st := r.getState() - scanner.PrintError(st.err, err) - st.exitCode = 2 + if err == errFormattingDiffers { + st.exitCode = 1 + } else { + scanner.PrintError(st.err, err) + st.exitCode = 2 + } } func (r *reporter) ExitCode() int { @@ -281,6 +288,7 @@ func processFile(filename string, info fs.FileInfo, in io.Reader, r *reporter) e newName := filepath.ToSlash(filename) oldName := newName + ".orig" r.Write(diff.Diff(oldName, src, newName, res)) + return errFormattingDiffers } } diff --git a/src/cmd/gofmt/gofmt_test.go b/src/cmd/gofmt/gofmt_test.go index 6b80673af148f5..eed05b55c2a6ed 100644 --- a/src/cmd/gofmt/gofmt_test.go +++ b/src/cmd/gofmt/gofmt_test.go @@ -10,6 +10,7 @@ import ( "internal/diff" "os" "path/filepath" + "strconv" "strings" "testing" "text/scanner" @@ -55,8 +56,10 @@ func gofmtFlags(filename string, maxLines int) string { func runTest(t *testing.T, in, out string) { // process flags + *doDiff = false *simplifyAST = false *rewriteRule = "" + exitCode := 0 info, err := os.Lstat(in) if err != nil { t.Error(err) @@ -72,6 +75,8 @@ func runTest(t *testing.T, in, out string) { switch name { case "": // no flags + case "-d": + *doDiff = true case "-r": *rewriteRule = value case "-s": @@ -79,6 +84,12 @@ func runTest(t *testing.T, in, out string) { case "-stdin": // fake flag - pretend input is from stdin info = nil + case "-exitcode": + // fake flag - set an expected exit code + exitCode, err = strconv.Atoi(value) + if err != nil { + t.Errorf("couldn't convert string to int: %s", value) + } default: t.Errorf("unrecognized flag name: %s", name) } @@ -96,8 +107,13 @@ func runTest(t *testing.T, in, out string) { if errBuf.Len() > 0 { t.Logf("%q", errBuf.Bytes()) } - if s.GetExitCode() != 0 { - t.Fail() + if s.GetExitCode() != exitCode { + t.Errorf("expected exit code %d, got %d", exitCode, s.GetExitCode()) + } + + // don't try to compare output for diffs + if *doDiff { + return } expected, err := os.ReadFile(out) @@ -140,6 +156,9 @@ func TestRewrite(t *testing.T) { t.Fatal(err) } + // add exit code tests + match = append(match, "testdata/exitcode0", "testdata/exitcode1") + // add larger examples match = append(match, "gofmt.go", "gofmt_test.go") diff --git a/src/cmd/gofmt/testdata/exitcode0 b/src/cmd/gofmt/testdata/exitcode0 new file mode 100644 index 00000000000000..2ba3e3a3de26b6 --- /dev/null +++ b/src/cmd/gofmt/testdata/exitcode0 @@ -0,0 +1,4 @@ +//gofmt -d -exitcode=0 + +// Input has correct formatting +package main diff --git a/src/cmd/gofmt/testdata/exitcode1 b/src/cmd/gofmt/testdata/exitcode1 new file mode 100644 index 00000000000000..251873b8a69917 --- /dev/null +++ b/src/cmd/gofmt/testdata/exitcode1 @@ -0,0 +1,4 @@ +//gofmt -d -exitcode=1 + +// Input has incorrect formatting + package main From 722b65dc38b7b1d9b3e0de9f1a083e542c4affe7 Mon Sep 17 00:00:00 2001 From: Robbie McMichael Date: Mon, 6 Oct 2025 17:46:03 +0800 Subject: [PATCH 3/4] Add a separate test function for the -d flag --- src/cmd/gofmt/gofmt_test.go | 78 ++++++++++++++++++++++---------- src/cmd/gofmt/testdata/exitcode0 | 2 - src/cmd/gofmt/testdata/exitcode1 | 2 - 3 files changed, 54 insertions(+), 28 deletions(-) diff --git a/src/cmd/gofmt/gofmt_test.go b/src/cmd/gofmt/gofmt_test.go index eed05b55c2a6ed..9eb9fa607321e1 100644 --- a/src/cmd/gofmt/gofmt_test.go +++ b/src/cmd/gofmt/gofmt_test.go @@ -10,7 +10,6 @@ import ( "internal/diff" "os" "path/filepath" - "strconv" "strings" "testing" "text/scanner" @@ -54,12 +53,19 @@ func gofmtFlags(filename string, maxLines int) string { return "" } -func runTest(t *testing.T, in, out string) { - // process flags - *doDiff = false - *simplifyAST = false +// Reset global variables for all flags to their default value. +func resetFlags() { + *list = false + *write = false *rewriteRule = "" - exitCode := 0 + *simplifyAST = false + *doDiff = false + *allErrors = false + *cpuprofile = "" +} + +func runTest(t *testing.T, in, out string) { + resetFlags() info, err := os.Lstat(in) if err != nil { t.Error(err) @@ -75,8 +81,6 @@ func runTest(t *testing.T, in, out string) { switch name { case "": // no flags - case "-d": - *doDiff = true case "-r": *rewriteRule = value case "-s": @@ -84,12 +88,6 @@ func runTest(t *testing.T, in, out string) { case "-stdin": // fake flag - pretend input is from stdin info = nil - case "-exitcode": - // fake flag - set an expected exit code - exitCode, err = strconv.Atoi(value) - if err != nil { - t.Errorf("couldn't convert string to int: %s", value) - } default: t.Errorf("unrecognized flag name: %s", name) } @@ -107,13 +105,8 @@ func runTest(t *testing.T, in, out string) { if errBuf.Len() > 0 { t.Logf("%q", errBuf.Bytes()) } - if s.GetExitCode() != exitCode { - t.Errorf("expected exit code %d, got %d", exitCode, s.GetExitCode()) - } - - // don't try to compare output for diffs - if *doDiff { - return + if s.GetExitCode() != 0 { + t.Fail() } expected, err := os.ReadFile(out) @@ -156,9 +149,6 @@ func TestRewrite(t *testing.T) { t.Fatal(err) } - // add exit code tests - match = append(match, "testdata/exitcode0", "testdata/exitcode1") - // add larger examples match = append(match, "gofmt.go", "gofmt_test.go") @@ -178,6 +168,46 @@ func TestRewrite(t *testing.T) { } } +// TestDiff runs gofmt with the -d flag on the input files and checks that the +// expected exit code is set. +func TestDiff(t *testing.T) { + tests := []struct { + in string + exitCode int + }{ + {in: "testdata/exitcode0", exitCode: 0}, + {in: "testdata/exitcode1", exitCode: 1}, + } + + for _, tt := range tests { + resetFlags() + *doDiff = true + + initParserMode() + initRewrite() + + info, err := os.Lstat(tt.in) + if err != nil { + t.Error(err) + return + } + + const maxWeight = 2 << 20 + var buf, errBuf bytes.Buffer + s := newSequencer(maxWeight, &buf, &errBuf) + s.Add(fileWeight(tt.in, info), func(r *reporter) error { + return processFile(tt.in, info, nil, r) + }) + if errBuf.Len() > 0 { + t.Logf("%q", errBuf.Bytes()) + } + + if s.GetExitCode() != tt.exitCode { + t.Errorf("%s: expected exit code %d, got %d", tt.in, tt.exitCode, s.GetExitCode()) + } + } +} + // Test case for issue 3961. func TestCRLF(t *testing.T) { const input = "testdata/crlf.input" // must contain CR/LF's diff --git a/src/cmd/gofmt/testdata/exitcode0 b/src/cmd/gofmt/testdata/exitcode0 index 2ba3e3a3de26b6..e46983b2349139 100644 --- a/src/cmd/gofmt/testdata/exitcode0 +++ b/src/cmd/gofmt/testdata/exitcode0 @@ -1,4 +1,2 @@ -//gofmt -d -exitcode=0 - // Input has correct formatting package main diff --git a/src/cmd/gofmt/testdata/exitcode1 b/src/cmd/gofmt/testdata/exitcode1 index 251873b8a69917..09cccdb9ab5c54 100644 --- a/src/cmd/gofmt/testdata/exitcode1 +++ b/src/cmd/gofmt/testdata/exitcode1 @@ -1,4 +1,2 @@ -//gofmt -d -exitcode=1 - // Input has incorrect formatting package main From db2207fba9a8f7a2f50138ec1f086ac6a74e1b10 Mon Sep 17 00:00:00 2001 From: Robbie McMichael Date: Mon, 6 Oct 2025 20:42:45 +0800 Subject: [PATCH 4/4] Rename files for exit code tests --- src/cmd/gofmt/gofmt_test.go | 4 ++-- src/cmd/gofmt/testdata/exitcode.golden | 1 + src/cmd/gofmt/testdata/exitcode.input | 1 + src/cmd/gofmt/testdata/exitcode0 | 2 -- src/cmd/gofmt/testdata/exitcode1 | 2 -- 5 files changed, 4 insertions(+), 6 deletions(-) create mode 100644 src/cmd/gofmt/testdata/exitcode.golden create mode 100644 src/cmd/gofmt/testdata/exitcode.input delete mode 100644 src/cmd/gofmt/testdata/exitcode0 delete mode 100644 src/cmd/gofmt/testdata/exitcode1 diff --git a/src/cmd/gofmt/gofmt_test.go b/src/cmd/gofmt/gofmt_test.go index 9eb9fa607321e1..2aba0f03ff09e9 100644 --- a/src/cmd/gofmt/gofmt_test.go +++ b/src/cmd/gofmt/gofmt_test.go @@ -175,8 +175,8 @@ func TestDiff(t *testing.T) { in string exitCode int }{ - {in: "testdata/exitcode0", exitCode: 0}, - {in: "testdata/exitcode1", exitCode: 1}, + {in: "testdata/exitcode.input", exitCode: 1}, + {in: "testdata/exitcode.golden", exitCode: 0}, } for _, tt := range tests { diff --git a/src/cmd/gofmt/testdata/exitcode.golden b/src/cmd/gofmt/testdata/exitcode.golden new file mode 100644 index 00000000000000..06ab7d0f9a35a7 --- /dev/null +++ b/src/cmd/gofmt/testdata/exitcode.golden @@ -0,0 +1 @@ +package main diff --git a/src/cmd/gofmt/testdata/exitcode.input b/src/cmd/gofmt/testdata/exitcode.input new file mode 100644 index 00000000000000..4f2f092ce508de --- /dev/null +++ b/src/cmd/gofmt/testdata/exitcode.input @@ -0,0 +1 @@ + package main diff --git a/src/cmd/gofmt/testdata/exitcode0 b/src/cmd/gofmt/testdata/exitcode0 deleted file mode 100644 index e46983b2349139..00000000000000 --- a/src/cmd/gofmt/testdata/exitcode0 +++ /dev/null @@ -1,2 +0,0 @@ -// Input has correct formatting -package main diff --git a/src/cmd/gofmt/testdata/exitcode1 b/src/cmd/gofmt/testdata/exitcode1 deleted file mode 100644 index 09cccdb9ab5c54..00000000000000 --- a/src/cmd/gofmt/testdata/exitcode1 +++ /dev/null @@ -1,2 +0,0 @@ -// Input has incorrect formatting - package main