On Jun 13 11:50:04, Jesus Sanchez wrote:
> Hi misc,
>
> First of all sorry for my crappy english, i'm trying my best:
>
> I found a posible issue with cp(1) when the source file and target file
> match
> with the process uid and gid and source file have S_ISUID or S_ISGID active.
> It
> overrides the target file permission mode with the source file mode. This
> issue
> also involves the '-p' behaviour. Before filling a bugreport I would like to
> discuss this in misc@
>
> In the form 'cp src dst', src and dst are existing regular files with same
> uid
> and gid with the calling process (i.e. user 'foo' copies in a file in his
> home), the mode of 'dst' will be the mode of 'src' with ~umask and
> RETAINBITS.
>
> the manpage says that a existing file will preserve mode
yes:
For each destination file that already exists, its contents are
overwritten if permissions allow, but its mode, user ID, and group ID are
unchanged.
> and if source file
> have set-user id bit or set-group id bit actives and both source file and
> target file share uid and gid with the calling process S_ISUID or S_ISGID
> bits
> will be preserved.
Actually, the manpage doesn't explicitly say "the set[ug]id bits will be
preserved if permissions allow"; it carefully describes the situations
when the source's setuid bit is *removed* from the copy (not the same owner,
or the owner cannot be preserved).
Anyway, the setuid/setgid bits are parts of the mode (right?), so they
should not be altered for *existing* destination files, according to the
quote above; which I suppose is what you are trying to say is broken.
> assuming user umask 0000
>
> if file 'src' exists with mode 4666 (rwSrw-rw-) and 'dst' exist with mode
> 0600
> (rw-------) the result of 'cp src dst' (if the process uid and gid is the
> same
> as the src and dst files) will be dst with 4666 instead 4600 as expected.
>
> In case of 'dst' beign an existing file it should preserve his original mode
> (0600) and add the S_ISUID or S_ISGID from the source file (4000) resulting
> in
> (4600), not (4666) as result.
>
> I wrote a dirty diff for src/bin/cp/util.c to illustrate the possible patch.
>
> to reproduce the issue:
>
> umask 000
> mkdir tmp
> cd tmp
> touch src
> touch dst
> chmod 4666 src
> cp src dst
> ls -l
> --- dst have 4666 mode (rwSrw-rw-) ---
I think I know what you mean, but there is at least the 'chmod 600 dst'
missing to show the behaviour you are describing. I will try to rephrase,
in context:
hans@stary:tmp$ cd `mktemp -d -p.`
hans@stary:tmp.7RPD6gvp99$ umask 000
hans@stary:tmp.7RPD6gvp99$ touch src
hans@stary:tmp.7RPD6gvp99$ ls -l
total 0
-rw-rw-rw- 1 hans wheel 0 Jun 13 17:19 src
hans@stary:tmp.7RPD6gvp99$ cp src dst
hans@stary:tmp.7RPD6gvp99$ ls -l
total 0
-rw-rw-rw- 1 hans wheel 0 Jun 13 17:19 dst
-rw-rw-rw- 1 hans wheel 0 Jun 13 17:19 src
Here, the destination file did not exist, so it was created
with the (umasked) mode of the source.
hans@stary:tmp.7RPD6gvp99$ chmod 600 dst
hans@stary:tmp.7RPD6gvp99$ ls -l
total 0
-rw------- 1 hans wheel 0 Jun 13 17:19 dst
-rw-rw-rw- 1 hans wheel 0 Jun 13 17:19 src
Now the destination file exists, so if we cp(1) again,
its mode should stay the same ...
hans@stary:tmp.7RPD6gvp99$ cp src dst
hans@stary:tmp.7RPD6gvp99$ ls -l
total 0
-rw------- 1 hans wheel 0 Jun 13 17:20 dst
-rw-rw-rw- 1 hans wheel 0 Jun 13 17:19 src
... and it does.
Now this is exactly what breaks if the src file has the setuid bit set:
hans@stary:tmp.7RPD6gvp99$ chmod 4666 src
hans@stary:tmp.7RPD6gvp99$ ls -l
total 0
-rw------- 1 hans wheel 0 Jun 13 17:20 dst
-rwSrw-rw- 1 hans wheel 0 Jun 13 17:19 src
hans@stary:tmp.7RPD6gvp99$ cp src dst
hans@stary:tmp.7RPD6gvp99$ ls -l
total 0
-rwSrw-rw- 1 hans wheel 0 Jun 13 17:20 dst
-rwSrw-rw- 1 hans wheel 0 Jun 13 17:19 src
Here, cp(1) did *not* preserve the mode of an existing destination file.
Is that what you meant?
(With regard to the '-p' option: with 'cp -p', the destination file's mode
is always overwritten by the (umasked) mode of the source, which is
what -p is supposed to do; so the above does not manifest with 'cp -p'.)
In short, it seems that "copying a setuid file implies -p".
-p Preserve in the copy as many of the modification time, access
time, file flags, file mode, user ID, and group ID as allowed by
permissions.
> [demime 1.01d removed an attachment of type application/octet-stream which
> had a name of 20110613_bin_cp_util_c.diff]
This maling list strips attachments; put your diff inline.
Jan
