[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[patch 2.1.0p1] filedir.c: improve setuid and setgid reporting

From: Sergio . Gelato
Subject: [patch 2.1.0p1] filedir.c: improve setuid and setgid reporting
Date: Sat, 13 Dec 2003 23:35:06 +0100
User-agent: Mutt/1.3.28i

The following patch may be somewhat controversial. I will apply it on my
systems, but other cfengine users may be of a different mind. The change
has security implications (hopefully for the better), so please think 

The patch does two things:
1) it gives cfengine a chance to report new setuid/setgid files in the
same run in which the set?id bit is turned on. Prior to the patch, one
had to wait for dstat->st_mode to have the bit turned on, which I think
postpones the warning until the next cfagent run (whenever that is;
maybe never).
2) it disables the reporting of setgid bits since that (a) has
doubtful usefulness, at least in the code's present form, and (b)
can suppress warnings when the setuid bit is subsequently turned on
on the same file.

For (2)(b), an alternative fix could be to introduce a VSETGIDLIST
distinct from VSETUIDLIST. Such a list should probably also keep track
of which group each file is setgid to. (Some groups, like "wheel" on
some systems, are more dangerous than others.)

--- orig/src/filedir.c
+++ mod/src/filedir.c
@@ -383,7 +383,7 @@
-if (dstat->st_uid == 0 && (dstat->st_mode & S_ISUID))
+if (dstat->st_uid == 0 && ((dstat->st_mode | newperm) & S_ISUID))
    if (newperm & S_ISUID)
@@ -424,7 +424,15 @@
-if (dstat->st_uid == 0 && (dstat->st_mode & S_ISGID))
+#if 0
+/* Disabled by Sergio Gelato, 2003-12-13:
+ * this code probably does more harm than good. The exploitability of a
+ * setgid executable for privilege escalation is independent of whether
+ * the file is owned by root (st_uid == 0). And by adding setgid-but-not-setuid
+ * executables to VSETUIDLIST one causes cfengine to remain silent if at a
+ * later time the setuid bit is turned on (see above).
+ */
+if (dstat->st_uid == 0 && ((dstat->st_mode | newperm) & S_ISGID))
    if (newperm & S_ISGID)
@@ -468,6 +476,7 @@
+#endif /* 2003-12-13: disable setgid check */
 if (CheckOwner(file,action,uidlist,gidlist,dstat))

reply via email to

[Prev in Thread] Current Thread [Next in Thread]