From c79a742fd1381a2ee32d28c5d97bbc19de34d54a Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Mon, 6 Feb 2023 12:16:47 -0500 Subject: [PATCH] unix: fix a use-after-free bug in PtraceIO on freebsd In CL 419915, both pointer fields of the PtraceIoDesc struct were converted to type uintptr to address golang/go#54113. However, that change was overzealous: the fix needed to convert fields that refer to addresses in the child process, but the Addr field of PtraceIoDesc is not even in the child process! It is instead an address in the parent (Go) process. Go's unsafe.Pointer rules prohibit converting a Go pointer to a uintptr except when immediately converting back to an unsafe.Pointer or calling a system call. Populating a PtraceIoDesc struct is neither of those things, so converting the Addr field to uintptr introduced a use-after-free bug. This change reverts the change to the Addr field from CL 419915 and consolidates the implementation of PtraceIO to reduce the the amount of code that varies with GOARCH. This change does not address the remaining ptrace uintptr bug (golang/go#58387), which is also present in the Linux implementation. Fixes golang/go#58351. Updates golang/go#54113. For golang/go#41205. Change-Id: I14bdb4af42130aa7b4375e3f53fd1a0435f14307 Reviewed-on: https://go-review.googlesource.com/c/sys/+/465676 Auto-Submit: Bryan Mills Run-TryBot: Bryan Mills Reviewed-by: Ian Lance Taylor TryBot-Result: Gopher Robot --- unix/mkpost.go | 6 ++++-- unix/syscall_freebsd.go | 19 +++++++++++++++++++ unix/syscall_freebsd_386.go | 15 ++++----------- unix/syscall_freebsd_amd64.go | 15 ++++----------- unix/syscall_freebsd_arm.go | 15 ++++----------- unix/syscall_freebsd_arm64.go | 15 ++++----------- unix/syscall_freebsd_riscv64.go | 15 ++++----------- unix/ztypes_freebsd_386.go | 2 +- unix/ztypes_freebsd_amd64.go | 2 +- unix/ztypes_freebsd_arm.go | 2 +- unix/ztypes_freebsd_arm64.go | 2 +- unix/ztypes_freebsd_riscv64.go | 2 +- 12 files changed, 48 insertions(+), 62 deletions(-) diff --git a/unix/mkpost.go b/unix/mkpost.go index 76075c34..783f11d9 100644 --- a/unix/mkpost.go +++ b/unix/mkpost.go @@ -89,10 +89,12 @@ func main() { return out }) - // Inside PtraceIoDesc replace all *byte with uintptr + // Inside PtraceIoDesc replace the Offs field, which refers to an address + // in the child process (not the Go parent), with a uintptr. ptraceIoDescStruct := regexp.MustCompile(`(?s:type PtraceIoDesc struct \{.*?\})`) + addrField := regexp.MustCompile(`(\bOffs\s+)\*byte`) b = ptraceIoDescStruct.ReplaceAllFunc(b, func(in []byte) []byte { - return bytes.ReplaceAll(in, []byte("*byte"), []byte("uintptr")) + return addrField.ReplaceAll(in, []byte(`${1}uintptr`)) }) } diff --git a/unix/syscall_freebsd.go b/unix/syscall_freebsd.go index d50b9dc2..19e71c22 100644 --- a/unix/syscall_freebsd.go +++ b/unix/syscall_freebsd.go @@ -274,6 +274,25 @@ func PtraceGetRegs(pid int, regsout *Reg) (err error) { return ptrace(PT_GETREGS, pid, uintptr(unsafe.Pointer(regsout)), 0) } +func PtraceIO(req int, pid int, offs uintptr, out []byte, countin int) (count int, err error) { + ioDesc := PtraceIoDesc{ + Op: int32(req), + Offs: offs, + } + if countin > 0 { + _ = out[:countin] // check bounds + ioDesc.Addr = &out[0] + } else if out != nil { + ioDesc.Addr = (*byte)(unsafe.Pointer(&_zero)) + } + ioDesc.SetLen(countin) + + // TODO(#58387): It's not actually safe to pass &ioDesc as a uintptr here. + // Pass it as an unsafe.Pointer instead. + err = ptrace(PT_IO, pid, uintptr(unsafe.Pointer(&ioDesc)), 0) + return int(ioDesc.Len), err +} + func PtraceLwpEvents(pid int, enable int) (err error) { return ptrace(PT_LWP_EVENTS, pid, 0, enable) } diff --git a/unix/syscall_freebsd_386.go b/unix/syscall_freebsd_386.go index 6a91d471..2f798dbe 100644 --- a/unix/syscall_freebsd_386.go +++ b/unix/syscall_freebsd_386.go @@ -42,6 +42,10 @@ func (cmsg *Cmsghdr) SetLen(length int) { cmsg.Len = uint32(length) } +func (d *PtraceIoDesc) SetLen(length int) { + d.Len = uint32(length) +} + func sendfile(outfd int, infd int, offset *int64, count int) (written int, err error) { var writtenOut uint64 = 0 _, _, e1 := Syscall9(SYS_SENDFILE, uintptr(infd), uintptr(outfd), uintptr(*offset), uintptr((*offset)>>32), uintptr(count), 0, uintptr(unsafe.Pointer(&writtenOut)), 0, 0) @@ -59,14 +63,3 @@ func Syscall9(num, a1, a2, a3, a4, a5, a6, a7, a8, a9 uintptr) (r1, r2 uintptr, func PtraceGetFsBase(pid int, fsbase *int64) (err error) { return ptrace(PT_GETFSBASE, pid, uintptr(unsafe.Pointer(fsbase)), 0) } - -func PtraceIO(req int, pid int, offs uintptr, out []byte, countin int) (count int, err error) { - ioDesc := PtraceIoDesc{ - Op: int32(req), - Offs: offs, - Addr: uintptr(unsafe.Pointer(&out[0])), // TODO(#58351): this is not safe. - Len: uint32(countin), - } - err = ptrace(PT_IO, pid, uintptr(unsafe.Pointer(&ioDesc)), 0) - return int(ioDesc.Len), err -} diff --git a/unix/syscall_freebsd_amd64.go b/unix/syscall_freebsd_amd64.go index 48110a0a..1ed93b69 100644 --- a/unix/syscall_freebsd_amd64.go +++ b/unix/syscall_freebsd_amd64.go @@ -42,6 +42,10 @@ func (cmsg *Cmsghdr) SetLen(length int) { cmsg.Len = uint32(length) } +func (d *PtraceIoDesc) SetLen(length int) { + d.Len = uint64(length) +} + func sendfile(outfd int, infd int, offset *int64, count int) (written int, err error) { var writtenOut uint64 = 0 _, _, e1 := Syscall9(SYS_SENDFILE, uintptr(infd), uintptr(outfd), uintptr(*offset), uintptr(count), 0, uintptr(unsafe.Pointer(&writtenOut)), 0, 0, 0) @@ -59,14 +63,3 @@ func Syscall9(num, a1, a2, a3, a4, a5, a6, a7, a8, a9 uintptr) (r1, r2 uintptr, func PtraceGetFsBase(pid int, fsbase *int64) (err error) { return ptrace(PT_GETFSBASE, pid, uintptr(unsafe.Pointer(fsbase)), 0) } - -func PtraceIO(req int, pid int, offs uintptr, out []byte, countin int) (count int, err error) { - ioDesc := PtraceIoDesc{ - Op: int32(req), - Offs: offs, - Addr: uintptr(unsafe.Pointer(&out[0])), // TODO(#58351): this is not safe. - Len: uint64(countin), - } - err = ptrace(PT_IO, pid, uintptr(unsafe.Pointer(&ioDesc)), 0) - return int(ioDesc.Len), err -} diff --git a/unix/syscall_freebsd_arm.go b/unix/syscall_freebsd_arm.go index 52f1d4b7..08932093 100644 --- a/unix/syscall_freebsd_arm.go +++ b/unix/syscall_freebsd_arm.go @@ -42,6 +42,10 @@ func (cmsg *Cmsghdr) SetLen(length int) { cmsg.Len = uint32(length) } +func (d *PtraceIoDesc) SetLen(length int) { + d.Len = uint32(length) +} + func sendfile(outfd int, infd int, offset *int64, count int) (written int, err error) { var writtenOut uint64 = 0 _, _, e1 := Syscall9(SYS_SENDFILE, uintptr(infd), uintptr(outfd), uintptr(*offset), uintptr((*offset)>>32), uintptr(count), 0, uintptr(unsafe.Pointer(&writtenOut)), 0, 0) @@ -55,14 +59,3 @@ func sendfile(outfd int, infd int, offset *int64, count int) (written int, err e } func Syscall9(num, a1, a2, a3, a4, a5, a6, a7, a8, a9 uintptr) (r1, r2 uintptr, err syscall.Errno) - -func PtraceIO(req int, pid int, offs uintptr, out []byte, countin int) (count int, err error) { - ioDesc := PtraceIoDesc{ - Op: int32(req), - Offs: offs, - Addr: uintptr(unsafe.Pointer(&out[0])), // TODO(#58351): this is not safe. - Len: uint32(countin), - } - err = ptrace(PT_IO, pid, uintptr(unsafe.Pointer(&ioDesc)), 0) - return int(ioDesc.Len), err -} diff --git a/unix/syscall_freebsd_arm64.go b/unix/syscall_freebsd_arm64.go index 5537ee4f..d151a0d0 100644 --- a/unix/syscall_freebsd_arm64.go +++ b/unix/syscall_freebsd_arm64.go @@ -42,6 +42,10 @@ func (cmsg *Cmsghdr) SetLen(length int) { cmsg.Len = uint32(length) } +func (d *PtraceIoDesc) SetLen(length int) { + d.Len = uint64(length) +} + func sendfile(outfd int, infd int, offset *int64, count int) (written int, err error) { var writtenOut uint64 = 0 _, _, e1 := Syscall9(SYS_SENDFILE, uintptr(infd), uintptr(outfd), uintptr(*offset), uintptr(count), 0, uintptr(unsafe.Pointer(&writtenOut)), 0, 0, 0) @@ -55,14 +59,3 @@ func sendfile(outfd int, infd int, offset *int64, count int) (written int, err e } func Syscall9(num, a1, a2, a3, a4, a5, a6, a7, a8, a9 uintptr) (r1, r2 uintptr, err syscall.Errno) - -func PtraceIO(req int, pid int, offs uintptr, out []byte, countin int) (count int, err error) { - ioDesc := PtraceIoDesc{ - Op: int32(req), - Offs: offs, - Addr: uintptr(unsafe.Pointer(&out[0])), // TODO(#58351): this is not safe. - Len: uint64(countin), - } - err = ptrace(PT_IO, pid, uintptr(unsafe.Pointer(&ioDesc)), 0) - return int(ioDesc.Len), err -} diff --git a/unix/syscall_freebsd_riscv64.go b/unix/syscall_freebsd_riscv64.go index 164abd5d..d5cd64b3 100644 --- a/unix/syscall_freebsd_riscv64.go +++ b/unix/syscall_freebsd_riscv64.go @@ -42,6 +42,10 @@ func (cmsg *Cmsghdr) SetLen(length int) { cmsg.Len = uint32(length) } +func (d *PtraceIoDesc) SetLen(length int) { + d.Len = uint64(length) +} + func sendfile(outfd int, infd int, offset *int64, count int) (written int, err error) { var writtenOut uint64 = 0 _, _, e1 := Syscall9(SYS_SENDFILE, uintptr(infd), uintptr(outfd), uintptr(*offset), uintptr(count), 0, uintptr(unsafe.Pointer(&writtenOut)), 0, 0, 0) @@ -55,14 +59,3 @@ func sendfile(outfd int, infd int, offset *int64, count int) (written int, err e } func Syscall9(num, a1, a2, a3, a4, a5, a6, a7, a8, a9 uintptr) (r1, r2 uintptr, err syscall.Errno) - -func PtraceIO(req int, pid int, offs uintptr, out []byte, countin int) (count int, err error) { - ioDesc := PtraceIoDesc{ - Op: int32(req), - Offs: offs, - Addr: uintptr(unsafe.Pointer(&out[0])), // TODO(#58351): this is not safe. - Len: uint64(countin), - } - err = ptrace(PT_IO, pid, uintptr(unsafe.Pointer(&ioDesc)), 0) - return int(ioDesc.Len), err -} diff --git a/unix/ztypes_freebsd_386.go b/unix/ztypes_freebsd_386.go index d9c78cdc..29dc4833 100644 --- a/unix/ztypes_freebsd_386.go +++ b/unix/ztypes_freebsd_386.go @@ -362,7 +362,7 @@ type FpExtendedPrecision struct{} type PtraceIoDesc struct { Op int32 Offs uintptr - Addr uintptr + Addr *byte Len uint32 } diff --git a/unix/ztypes_freebsd_amd64.go b/unix/ztypes_freebsd_amd64.go index 26991b16..0a89b289 100644 --- a/unix/ztypes_freebsd_amd64.go +++ b/unix/ztypes_freebsd_amd64.go @@ -367,7 +367,7 @@ type FpExtendedPrecision struct{} type PtraceIoDesc struct { Op int32 Offs uintptr - Addr uintptr + Addr *byte Len uint64 } diff --git a/unix/ztypes_freebsd_arm.go b/unix/ztypes_freebsd_arm.go index f8324e7e..c8666bb1 100644 --- a/unix/ztypes_freebsd_arm.go +++ b/unix/ztypes_freebsd_arm.go @@ -350,7 +350,7 @@ type FpExtendedPrecision struct { type PtraceIoDesc struct { Op int32 Offs uintptr - Addr uintptr + Addr *byte Len uint32 } diff --git a/unix/ztypes_freebsd_arm64.go b/unix/ztypes_freebsd_arm64.go index 4220411f..88fb48a8 100644 --- a/unix/ztypes_freebsd_arm64.go +++ b/unix/ztypes_freebsd_arm64.go @@ -347,7 +347,7 @@ type FpExtendedPrecision struct{} type PtraceIoDesc struct { Op int32 Offs uintptr - Addr uintptr + Addr *byte Len uint64 } diff --git a/unix/ztypes_freebsd_riscv64.go b/unix/ztypes_freebsd_riscv64.go index 0660fd45..698dc975 100644 --- a/unix/ztypes_freebsd_riscv64.go +++ b/unix/ztypes_freebsd_riscv64.go @@ -348,7 +348,7 @@ type FpExtendedPrecision struct{} type PtraceIoDesc struct { Op int32 Offs uintptr - Addr uintptr + Addr *byte Len uint64 }