[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: 1.25.1 cpio Extract Issue
From:       Marc Smith <marc.smith () mcc ! edu>
Date:       2016-10-31 14:04:19
Message-ID: CAHkw+Lc=gFvf2wKbUiX6ti5bdfjkfqeLLsevt9PY6kyKqy7=Pg () mail ! gmail ! com
[Download RAW message or body]

I tested my theory by initializing archive_handle->cpio__owner and it
now behaves correctly when using the extract mode without "-R" -- it
keeps the original UID/GID from the archive. I don't know enough about
the rest of the program to say whether this is safe and/or correct,
but I tested with the following patch:

--- a/archival/cpio.c   2016-10-07 10:47:47.000000000 -0400
+++ b/archival/cpio.c   2016-10-31 09:57:19.661157962 -0400
@@ -383,6 +383,8 @@
        archive_handle = init_handle();
        /* archive_handle->src_fd = STDIN_FILENO; - done by init_handle */
        archive_handle->ah_flags = ARCHIVE_EXTRACT_NEWER;
+       archive_handle->cpio__owner.uid = -1L;
+       archive_handle->cpio__owner.gid = -1L;

        /* As of now we do not enforce this: */
        /* -i,-t,-o,-p are mutually exclusive */


--Marc

On Sun, Oct 30, 2016 at 12:33 PM, Marc Smith <marc.smith@mcc.edu> wrote:
> Hi,
>
> I recently updated my project to use Busybox version 1.25.1. After
> doing so, I noticed that archives extracted with BB's cpio were all
> owned by root (when executing cpio as root). I'm not using the newish
> "-R" option.
>
> I believe this problem is caused by commit
> 2735bc00e35c5fd8eec6d656f4d8a17ee2630c2a
> (https://git.busybox.net/busybox/commit/?id=2735bc00e35c5fd8eec6d656f4d8a17ee2630c2a)
> - "cpio: implement -R/--owner".
>
> I quickly looked at the source this morning, and after adding a couple
> monitoring lines, it looks like this is the culprit
> (archival/libarchive/get_header_cpio.c lines 55-59 of 1.25.1):
>
> --snip--
>     /* "cpio -R USER:GRP" support: */
>     if (archive_handle->cpio__owner.uid != (uid_t)-1L)
>         uid = archive_handle->cpio__owner.uid;
>     if (archive_handle->cpio__owner.gid != (gid_t)-1L)
>         gid = archive_handle->cpio__owner.gid;
> --snip--
>
> When testing, archive_handle->cpio__owner.uid and
> archive_handle->cpio__owner.gid have a value of 0, when I believe by
> default it should have a value of -1 if the "-R" option is not used.
> So, instead of using the value from the header, the UID and GID always
> set to 0.
>
> I think the proper fix would be to initialize
> archive_handle->cpio__owner.uid and archive_handle->cpio__owner.gid to
> -1L and then if "-R" is used it gets overridden. I'm just not sure of
> the correct place to initialize these.
>
>
> Thanks,
>
> Marc
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]