This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Declare set*id with warn_unused_result


On Linux (except very current versions without funky security modules), set*uid can fail with EAGAIN when RLIMIT_NPROC would be exceeded. Missing return value checks are known to result in privilege escalation vulnerabilities. It is a common coding error to call setuid before setgid, so that the setgid fails, and checking for the setgid result should prevent this mistake from going unnoticed. Therefore, I think it makes sense to add the attribute to both groups of functions.

With this patch, I get a few new warnings in libc itself, in the nscd directory:

connections.c:1470:11: warning: ignoring return value of ‘setuid’, declared with attribute warn_unused_result [-Wunused-result]
connections.c:1485:11: warning: ignoring return value of ‘setuid’, declared with attribute warn_unused_result [-Wunused-result]
connections.c:1486:11: warning: ignoring return value of ‘setgid’, declared with attribute warn_unused_result [-Wunused-result]
connections.c:1530:14: warning: ignoring return value of ‘setuid’, declared with attribute warn_unused_result [-Wunused-result]
connections.c:1531:14: warning: ignoring return value of ‘setgid’, declared with attribute warn_unused_result [-Wunused-result]


I'm not sure what the code is trying to do. Testing with "paranoia yes" suggests that it's probably harmless, but it would be great if someone could take a closer look.

--
Florian Weimer / Red Hat Product Security Team

commit 0816aa75de295b2c1be78393b4dbd6b96105f827
Author: Florian Weimer <fweimer@redhat.com>
Date:   Tue Jul 24 13:45:59 2012 +0200

    	* posix/unistd.h (setuid, setreuid, seteuid, setresuid):
    	Declare with warn_unused_result.
    	(setgid, setregid, setegid, setresgid): Likewise.
    	* sysdeps/unix/sysv/linux/sys/fsuid.h (setfsuid, setfsgid):
    	Likewise.
    	* WUR-REPORT: Remove set*id functions.

diff --git a/ChangeLog b/ChangeLog
index f9f0e44..8a4fe6c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2012-07-24  Florian Weimer  <fweimer@redhat.com>
+
+	* posix/unistd.h (setuid, setreuid, seteuid, setresuid):
+	Declare with warn_unused_result.
+	(setgid, setregid, setegid, setresgid): Likewise.
+	* sysdeps/unix/sysv/linux/sys/fsuid.h (setfsuid, setfsgid):
+	Likewise.
+	* WUR-REPORT: Remove set*id functions.
+
 2012-07-23  Thomas Schwinge  <thomas@codesourcery.com>
 
 	* configure.in <sysdeps resolving>: Correct printing
diff --git a/WUR-REPORT b/WUR-REPORT
index ef407cf..d997bd0 100644
--- a/WUR-REPORT
+++ b/WUR-REPORT
@@ -4,17 +4,6 @@ lssek:   Probably should be __wur but lseek(fd,SEEK_SET,0) will succeed if
          the descriptor is fine.
 lseek64: same
 
-setuid:  will always succeed given correct privileges, so there might
-         be places which don't check for it.
-setreuid: same
-seteuid: same
-setgid:  same
-setregid: same
-setegid: same
-setresuid: same
-setresgid: same
-
-
 <stdio.h>:
 
 setvbuf:   if stream and buffer are fine and other parameters constant,
diff --git a/posix/unistd.h b/posix/unistd.h
index 9839761..88d711a 100644
--- a/posix/unistd.h
+++ b/posix/unistd.h
@@ -719,34 +719,34 @@ extern int group_member (__gid_t __gid) __THROW;
    If the calling process is the super-user, set the real
    and effective user IDs, and the saved set-user-ID to UID;
    if not, the effective user ID is set to UID.  */
-extern int setuid (__uid_t __uid) __THROW;
+extern int setuid (__uid_t __uid) __THROW __wur;
 
 #if defined __USE_BSD || defined __USE_XOPEN_EXTENDED
 /* Set the real user ID of the calling process to RUID,
    and the effective user ID of the calling process to EUID.  */
-extern int setreuid (__uid_t __ruid, __uid_t __euid) __THROW;
+extern int setreuid (__uid_t __ruid, __uid_t __euid) __THROW __wur;
 #endif
 
 #if defined __USE_BSD || defined __USE_XOPEN2K
 /* Set the effective user ID of the calling process to UID.  */
-extern int seteuid (__uid_t __uid) __THROW;
+extern int seteuid (__uid_t __uid) __THROW __wur;
 #endif /* Use BSD.  */
 
 /* Set the group ID of the calling process to GID.
    If the calling process is the super-user, set the real
    and effective group IDs, and the saved set-group-ID to GID;
    if not, the effective group ID is set to GID.  */
-extern int setgid (__gid_t __gid) __THROW;
+extern int setgid (__gid_t __gid) __THROW __wur;
 
 #if defined __USE_BSD || defined __USE_XOPEN_EXTENDED
 /* Set the real group ID of the calling process to RGID,
    and the effective group ID of the calling process to EGID.  */
-extern int setregid (__gid_t __rgid, __gid_t __egid) __THROW;
+extern int setregid (__gid_t __rgid, __gid_t __egid) __THROW __wur;
 #endif
 
 #if defined __USE_BSD || defined __USE_XOPEN2K
 /* Set the effective group ID of the calling process to GID.  */
-extern int setegid (__gid_t __gid) __THROW;
+extern int setegid (__gid_t __gid) __THROW __wur;
 #endif /* Use BSD.  */
 
 #ifdef __USE_GNU
@@ -763,12 +763,12 @@ extern int getresgid (__gid_t *__rgid, __gid_t *__egid, __gid_t *__sgid)
 /* Set the real user ID, effective user ID, and saved-set user ID,
    of the calling process to RUID, EUID, and SUID, respectively.  */
 extern int setresuid (__uid_t __ruid, __uid_t __euid, __uid_t __suid)
-     __THROW;
+     __THROW __wur;
 
 /* Set the real group ID, effective group ID, and saved-set group ID,
    of the calling process to RGID, EGID, and SGID, respectively.  */
 extern int setresgid (__gid_t __rgid, __gid_t __egid, __gid_t __sgid)
-     __THROW;
+     __THROW __wur;
 #endif
 
 
diff --git a/sysdeps/unix/sysv/linux/sys/fsuid.h b/sysdeps/unix/sysv/linux/sys/fsuid.h
index 2fd512e..4494baf 100644
--- a/sysdeps/unix/sysv/linux/sys/fsuid.h
+++ b/sysdeps/unix/sysv/linux/sys/fsuid.h
@@ -25,10 +25,10 @@ __BEGIN_DECLS
 
 /* Change uid used for file access control to UID, without affecting
    other privileges (such as who can send signals at the process).  */
-extern int setfsuid (__uid_t __uid) __THROW;
+extern int setfsuid (__uid_t __uid) __THROW __wur;
 
 /* Ditto for group id. */
-extern int setfsgid (__gid_t __gid) __THROW;
+extern int setfsgid (__gid_t __gid) __THROW __wur;
 
 __END_DECLS
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]