On Mon, 23 Feb 2004, Andrew Pimlott wrote:
> - If tcpdump is setuid root, "tcpdump -Z root" enables anyone to read
> and write root's files, as well as get root from any exploit.
Fixed in the latest proposed patch, if adopted.
> - If root uses "tcpdump -Z nobody", he will not be able to read his own
> files with "-r" (my first patch had the same issue). I don't think
> this is desirable.
Root will always be able to read files written by nobody, right? :)
> He will also not be able to write his own files
> with "-w", and this problem existed in my patch as well. The simplest
> solution would seem to be doing the "-w" earlier, but I'm not sure.
> (This seems also to apply to -F, and perhaps something else I've
> missed in a quick scan of what happens after -Z is handled.)
Yep, this is a problem. Quickest fix is delaying dropping the
privileges, as done in the patch I posted yesterday.
> - It doesn't make sense for WITH_USER to be handled so much later than
> -Z. Perhaps the author noticed the above problems and decided to drop
> privileges later. Ok, but then -Z should be done later too.
The distinction was made because some vendors drop the privileges by
default, and don't expect to see warnings about write files failing.
On the other hand, if the user uses -Z himself, he can expect some
trouble. But I agree that this is surprising, and should be changed,
as done in the latest patch.
> - initgroups(pw->pw_name, 0) causes gid 0 to be left in the supplemental
> group list. It should be initgroups(pw->pw_name, pw->pw_gid).
Hmm; this seems to be the case. I vaguely recall that there would be
some reason why this would not be needed. But it seems this is needed
after all.. attached fixes this, naturally..
--
Pekka Savola "You each name yourselves king, yet the
Netcore Oy kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings
--- tcpdump.c~ Tue Feb 24 08:59:43 2004
+++ tcpdump.c Tue Feb 24 08:59:43 2004
@@ -332,7 +332,7 @@
pw = getpwnam(username);
if (pw) {
- if (initgroups(pw->pw_name, 0) != 0 || setgid(pw->pw_gid) != 0 ||
+ if (initgroups(pw->pw_name, pw->pw_gid) != 0 || setgid(pw->pw_gid) !=
0 ||
setuid(pw->pw_uid) != 0) {
fprintf(stderr, "Couldn't change to '%.32s' uid=%d gid=%d\n",
username,
pw->pw_uid, pw->pw_gid);
