[ovs-dev] [PATCH v2] lib: protect daemon_set_new_user against non existing user:group specs

Christian Ehrhardt christian.ehrhardt at canonical.com
Mon Apr 25 07:12:19 UTC 2016


>From the manpages of getgrnam_r (getpwnam_r is similar):
"If no matching group record was found, these functions return 0 and
store NULL in *result."

The code checked only against errors, but non existing users didn't set
e != 0 therefore the code could try to set arbitrary uid/gid values.

*Update in v2*
fix wrong pointer usage of *res and running full set of unit tests to be sure.

Fixes: e91b927d lib/daemon: support --user option for all OVS daemon

Signed-off-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
---
 lib/daemon-unix.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
index 182f76b..28f76da 100644
--- a/lib/daemon-unix.c
+++ b/lib/daemon-unix.c
@@ -972,6 +972,9 @@ daemon_set_new_user(const char *user_spec)
             VLOG_FATAL("%s: Failed to retrive user %s's uid (%s), aborting.",
                        pidfile, user, ovs_strerror(e));
         }
+        if (res == NULL) {
+            VLOG_FATAL("%s: user %s not found, aborting.", pidfile, user);
+        }
     } else {
         /* User name is not specified, use current user.  */
         while ((e = getpwuid_r(uid, &pwd, buf, bufsize, &res)) == ERANGE) {
@@ -1012,6 +1015,10 @@ daemon_set_new_user(const char *user_spec)
                            "(%s), aborting.", pidfile, grpstr,
                            ovs_strerror(e));
             }
+            if (res == NULL) {
+                VLOG_FATAL("%s: group %s not found, aborting.", pidfile,
+                           grpstr);
+            }
 
             if (gid != grp.gr_gid) {
                 char **mem;
-- 
2.7.4




More information about the dev mailing list