[Bug binutils/26945] Unsafe chown+chmod in smart_rename, possibly elsewhere

[siddhesh at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26945

Siddhesh Poyarekar <siddhesh at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #15 from Siddhesh Poyarekar <siddhesh at sourceware dot org> ---
This is fixed in master:

commit 014cc7f849e8209623fc99264814bce7b3b6faf2 (origin/master, origin/HEAD)
Author: Siddhesh Poyarekar <siddhesh@gotplt.org>
Date:   Mon Dec 7 20:48:33 2020 +0530

    binutils: Make smart_rename safe too

    smart_rename is capable of handling symlinks by copying and it also
    tries to preserve ownership and permissions of files when they're
    overwritten during the rename.  This is useful in objcopy where the
    file properties need to be preserved.

    However because smart_rename does this using file names, it leaves a
    race window between renames and permission fixes.  This change removes
    this race window by using file descriptors from the original BFDs that
    were used to manipulate these files wherever possible.

    The file that is to be renamed is also passed as a file descriptor so
    that we use fchown/fchmod on the file descriptor, thus making sure
    that we only modify the file we have opened to write.  Further, in
    case the file is to be overwritten (as is the case in ar or objcopy),
    the permissions that need to be restored are taken from the file
    descriptor that was opened for input so that integrity of the file
    status is maintained all the way through to the rename.

    binutils/

            * rename.c
            * ar.c
            (write_archive) [!defined (_WIN32) || defined (__CYGWIN32__)]:
            Initialize TARGET_STAT and OFD to pass to SMART_RENAME.
            * arsup.c
            (ar_save) [defined (_WIN32) || defined (__CYGWIN32__)]:
            Likewise.
            * bucomm.h (smart_rename): Add new arguments to declaration.
            * objcopy.c
            (strip_main)[defined (_WIN32) || defined (__CYGWIN32__)]:
            Initialize COPYFD and pass to SMART_RENAME.
            (copy_main) [defined (_WIN32) || defined (__CYGWIN32__)]:
            Likewise.
            * rename.c (try_preserve_permissions): New function.
            (smart_rename): Use it and add new arguments.

commit 1a1c3b4cc17687091cff5a368bd6f13742bcfdf8
Author: Siddhesh Poyarekar <siddhesh@gotplt.org>
Date:   Mon Dec 7 20:48:28 2020 +0530

    objcopy: Get input file stat after BFD open

    Get file state from the descriptor opened by copy_file for the input
    BFD.  This ensures continuity in the view of the input file through
    the descriptor.  At the moment it is only to preserve timestamps
    recorded at the point that we opened the file for input but in the
    next patch this state will also be used to preserve ownership and
    permissions wherever applicable.

    binutils/

            * objcopy.c (copy_file): New argument IN_STAT.  Return stat of
            ibfd through it.
            (strip_main): Remove redundant stat calls.  adjust copy_file
            calls.
            (copy_main): Likewise.

commit 365f5fb6d0f0da83817431a275e99e6f6babbe04
Author: Siddhesh Poyarekar <siddhesh@gotplt.org>
Date:   Mon Dec 7 20:48:23 2020 +0530

    binutils: Use file descriptors from make_tempname

    The purpose of creating a temporary file securely using mkstemp is
    defeated if it is closed in make_tempname and reopened later for use;
    it is as good as using mktemp.  Get the file descriptor instead and
    then use it to create the BFD object.

    bfd/

            * opncls.c (bfd_fdopenw): New function.
            * bfd-in2.h: Regenerate.

    binutils/

            * bucomm.c (make_tempname): Add argument to return file
            descriptor.
            * bucomm.h (make_tempname): Likewise.
            * ar.c: Include libbfd.h.
            (write_archive): Adjust for change in make_tempname.  Call
            bfd_fdopenw instead of bfd_openw.
            * objcopy.c: Include libbfd.h.
            (copy_file): New argument OFD.  Use bfd_fdopenw instead of
            bfd_openw.
            (strip_main): Adjust for change in make_tempname and
            copy_file.
            (copy_main): Likewise.

-- 
You are receiving this mail because:
You are on the CC list for the bug.