diff --git a/pkg/sentry/socket/hostinet/socket_unsafe.go b/pkg/sentry/socket/hostinet/socket_unsafe.go index af7327428..36339ac08 100644 --- a/pkg/sentry/socket/hostinet/socket_unsafe.go +++ b/pkg/sentry/socket/hostinet/socket_unsafe.go @@ -97,10 +97,24 @@ func ioctl(ctx context.Context, fd int, io usermem.IO, args arch.SyscallArgument if _, err := ifc.CopyIn(cc, args[2].Pointer()); err != nil { return 0, err } - // TODO(b/209503078): Check ifc.Ptr range is in untrusted range. - if _, _, errno := unix.Syscall(unix.SYS_IOCTL, uintptr(fd), cmd, uintptr(unsafe.Pointer(&ifc))); errno != 0 { + + // The user's ifconf can have a nullable pointer to a buffer. Use a Sentry array if non-null. + ifcNested := linux.IFConf{Len: ifc.Len} + var ifcBuf []byte + if ifc.Ptr != 0 { + ifcBuf = make([]byte, ifc.Len) + ifcNested.Ptr = uint64(uintptr(unsafe.Pointer(&ifcBuf[0]))) + } + if _, _, errno := unix.Syscall(unix.SYS_IOCTL, uintptr(fd), cmd, uintptr(unsafe.Pointer(&ifcNested))); errno != 0 { return 0, translateIOSyscallError(errno) } + // Copy out the buffer if it was non-null. + if ifc.Ptr != 0 { + if _, err := cc.CopyOutBytes(hostarch.Addr(ifc.Ptr), ifcBuf); err != nil { + return 0, err + } + } + ifc.Len = ifcNested.Len _, err := ifc.CopyOut(cc, args[2].Pointer()) return 0, err case linux.SIOCETHTOOL: diff --git a/test/syscalls/BUILD b/test/syscalls/BUILD index 7b6bbb3a6..fc8f9a1e8 100644 --- a/test/syscalls/BUILD +++ b/test/syscalls/BUILD @@ -241,6 +241,7 @@ syscall_test( syscall_test( size = "medium", + add_hostinet = True, add_overlay = True, test = "//test/syscalls/linux:ioctl_test", ) diff --git a/test/syscalls/linux/ioctl.cc b/test/syscalls/linux/ioctl.cc index 88056ef2e..5a2f8283e 100644 --- a/test/syscalls/linux/ioctl.cc +++ b/test/syscalls/linux/ioctl.cc @@ -350,6 +350,22 @@ TEST_P(IoctlTestSIOCGIFCONF, ValidateNoPartialIfrsReturned) { ASSERT_NE(ifr.ifr_name[0], '\0'); // An interface can now be returned. } +// This test validates that nested pointers aren't allowed to escape the +// address space. +TEST_P(IoctlTestSIOCGIFCONF, ValidateNestedPointerCheck) { + auto fd = ASSERT_NO_ERRNO_AND_VALUE(NewSocket()); + + struct ifconf ifconf = {}; + ifconf.ifc_len = sizeof(ifreq); + // Address chosen with ASLR disabled, pausing here, and inspecting the + // process with /proc//maps to find a writable mapping in the low range + // of gr0 memory. + ifconf.ifc_ifcu.ifcu_req = reinterpret_cast(0x3f9000d51000); + + ASSERT_THAT(ioctl(fd->get(), SIOCGIFCONF, &ifconf), + SyscallFailsWithErrno(EFAULT)); +} + TEST_P(IoctlTestSIOCGIFCONF, ValidateLoopbackIsPresent) { auto fd = ASSERT_NO_ERRNO_AND_VALUE(NewSocket());