diff --git a/tools/tht/internal/safeio/preflight_windows.go b/tools/tht/internal/safeio/preflight_windows.go index 57a6989d..fe571bdb 100644 --- a/tools/tht/internal/safeio/preflight_windows.go +++ b/tools/tht/internal/safeio/preflight_windows.go @@ -16,15 +16,17 @@ func preflightPrivateDirectory(path string) (bool, error) { handle, err := openWindowsRelativeComponent(parents.handles[len(parents.handles)-1], target, true, windows.GENERIC_READ) if err != nil { if isWindowsRelativeNotFound(err) { - writableParent, accessErr := openWindowsComponentWithAccess( - parents.directory, - true, + // A read-only retained handle cannot be upgraded. Re-traverse with the + // directory FILE_ADD_SUBDIRECTORY right, still using RootDirectory-relative + // opens for every component rather than reopening parents by absolute path. + writableParents, _, accessErr := openCanonicalWindowsParentWithFinalAccess( + path, windows.FILE_APPEND_DATA, // FILE_ADD_SUBDIRECTORY for a directory handle ) - if accessErr != nil { + if accessErr != nil || writableParents == nil { return false, ErrUnsafeFile } - _ = windows.CloseHandle(writableParent) + writableParents.Close() return false, nil } return false, ErrUnsafeFile diff --git a/tools/tht/internal/safeio/private_root_windows.go b/tools/tht/internal/safeio/private_root_windows.go index 2d38e11a..0a3181b7 100644 --- a/tools/tht/internal/safeio/private_root_windows.go +++ b/tools/tht/internal/safeio/private_root_windows.go @@ -48,26 +48,22 @@ func openPrivateDirectory(path string, ensure bool) (PrivateDirectoryHandle, boo // The final root is absent. The retained canonical parent must prove that the same // ensure operation could create it; validation itself must remain side-effect free. // FILE_APPEND_DATA is the Win32 spelling of directory FILE_ADD_SUBDIRECTORY. - probe, probeErr := openWindowsComponentWithAccess(anchors.directory, true, windows.FILE_APPEND_DATA) - if probeErr != nil { - anchors.Close() + anchors.Close() + writableAnchors, writableTarget, probeErr := openCanonicalWindowsParentWithFinalAccess(path, windows.FILE_APPEND_DATA) + if probeErr != nil || writableAnchors == nil || len(writableAnchors.handles) == 0 { + if writableAnchors != nil { + writableAnchors.Close() + } return nil, false, ErrUnsafeFile } - _ = windows.CloseHandle(probe) if !ensure { - anchors.Close() + writableAnchors.Close() return nil, false, nil } - // The canonical parent chain remains pinned by anchors; this extra handle supplies - // FILE_ADD_SUBDIRECTORY for the one initial root creation without reopening a child - // beneath the private root lexically. - writableParent, writableErr := openWindowsComponentWithAccess(anchors.directory, true, windows.FILE_APPEND_DATA) - if writableErr != nil { - anchors.Close() - return nil, false, ErrUnsafeFile - } - handle, found, err = openWindowsPrivateDirectoryAt(writableParent, target, true) - _ = windows.CloseHandle(writableParent) + anchors = writableAnchors + target = writableTarget + parent = anchors.handles[len(anchors.handles)-1] + handle, found, err = openWindowsPrivateDirectoryAt(parent, target, true) if err != nil || !found { anchors.Close() return nil, false, ErrUnsafeFile diff --git a/tools/tht/internal/safeio/private_windows.go b/tools/tht/internal/safeio/private_windows.go index 5376aeda..b8af531a 100644 --- a/tools/tht/internal/safeio/private_windows.go +++ b/tools/tht/internal/safeio/private_windows.go @@ -13,7 +13,7 @@ import ( ) func createPrivateDirectory(path string) error { - parents, target, err := openCanonicalWindowsParent(path) + parents, target, err := openCanonicalWindowsParentWithFinalAccess(path, windows.FILE_APPEND_DATA) if err != nil || parents == nil || len(parents.handles) == 0 { if parents != nil { parents.Close() @@ -112,7 +112,10 @@ func createCanonicalNewPrivateParentReadWriteFile(path string, mode os.FileMode) } func createCanonicalNewFile(path string, mode os.FileMode, requirePrivateParent bool, access uint32) (*os.File, error) { - parents, target, err := openCanonicalWindowsParent(path) + // FILE_WRITE_DATA is FILE_ADD_FILE when the retained handle names a directory. Request it + // while that final parent is opened, rather than reopening its lexical path later to gain + // create permission. + parents, target, err := openCanonicalWindowsParentWithFinalAccess(path, windows.FILE_WRITE_DATA) if err != nil || parents == nil || len(parents.handles) == 0 || (requirePrivateParent && validateOwnerOnlyDACL(parents.handles[len(parents.handles)-1]) != nil) { if parents != nil { parents.Close() @@ -172,6 +175,13 @@ func (parents *windowsParentHandles) Close() { // target parent without FILE_SHARE_DELETE. Every component after the volume root is resolved // through the prior retained handle's NT RootDirectory, never by re-opening an absolute prefix. func openCanonicalWindowsParent(path string) (*windowsParentHandles, string, error) { + return openCanonicalWindowsParentWithFinalAccess(path, 0) +} + +// openCanonicalWindowsParentWithFinalAccess gives only the final retained parent the requested +// child-operation capability. It is the Windows openat traversal for a later relative create; +// reopening that parent by its reconstructed path would recreate the ancestor-swap race. +func openCanonicalWindowsParentWithFinalAccess(path string, finalParentAccess uint32) (*windowsParentHandles, string, error) { if err := ValidateCanonicalPath(path); err != nil { return nil, "", err } @@ -182,13 +192,21 @@ func openCanonicalWindowsParent(path string) (*windowsParentHandles, string, err return nil, "", ErrUnsafeFile } parents := &windowsParentHandles{directory: root} - rootHandle, err := openWindowsComponent(root, true) + rootAccess := uint32(windows.GENERIC_READ) + if len(components) == 1 { + rootAccess |= finalParentAccess + } + rootHandle, err := openWindowsComponentWithAccess(root, true, rootAccess) if err != nil { return nil, "", err } parents.handles = append(parents.handles, rootHandle) - for _, component := range components[:len(components)-1] { - handle, err := openWindowsRelativeComponent(parents.handles[len(parents.handles)-1], component, true, windows.GENERIC_READ) + for index, component := range components[:len(components)-1] { + componentAccess := uint32(windows.GENERIC_READ) + if index == len(components)-2 { + componentAccess |= finalParentAccess + } + handle, err := openWindowsRelativeComponent(parents.handles[len(parents.handles)-1], component, true, componentAccess) if err != nil { parents.Close() return nil, "", err