From af9e3f8c8d6a49bd4253e0b125d693a33ec253f0 Mon Sep 17 00:00:00 2001 From: Miles Bader Date: Thu, 28 Dec 1995 22:36:54 +0000 Subject: (setid): Don't touch the return params unless we succeed. Add SETID parameter, and just copy old into new unless it's set. Handle the NOLDGENIDS == 0 case correctly. (diskfs_S_file_exec): Use the new setid() properly. Make sure that {GEN,AUX}{UIDS,GIDS} are always in a state where they can be freed. --- libdiskfs/file-exec.c | 224 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 133 insertions(+), 91 deletions(-) (limited to 'libdiskfs/file-exec.c') diff --git a/libdiskfs/file-exec.c b/libdiskfs/file-exec.c index 97976bf4..fbe0e11e 100644 --- a/libdiskfs/file-exec.c +++ b/libdiskfs/file-exec.c @@ -37,93 +37,138 @@ scan_ids (uid_t *set, int setlen, uid_t test) return 0; } -/* Adds ID to the sets OLDGENIDS & OLDAUXIDS, and returns the new set in - GENIDS and AUXIDS, which are malloced. SECURE is also updated to reflect - whether a secure exec is called for. ENOMEM is returned if a malloc - fails, otherwise 0. */ +/* If SETID is true, adds ID to the sets OLDGENIDS & OLDAUXIDS, and returns + the new set in GENIDS and AUXIDS, which are malloced; if SETID is false, + just makes GENIDS and AUXIDS (malloced) copies of OLDGENIDS & OLDAUXIDS; + SECURE is also updated to reflect whether a secure exec is called for. + ENOMEM is returned if a malloc fails, otherwise 0. The return parameters + are only touched if this call succeeds. */ static error_t -setid (uid_t id, int *secure, +setid (int setid, uid_t id, int *secure, uid_t *oldgenids, size_t noldgenids, uid_t *oldauxids, size_t noldauxids, uid_t **genids, size_t *ngenids, uid_t **auxids, size_t *nauxids) { - /* We are dumping the current first id; put it - into the auxids array. This is complex. The different - cases below are intended to make sure that we don't - lose any ids (unlike posix) and to make sure that aux ids - zero and one (if already set) behave like the posix - ones. */ + /* These hold the new auxids array until we're sure we're going to return. */ + uid_t *_genids, *_auxids; + size_t _ngenids, _nauxids; -#define MALLOC(n) ({ void *_p = malloc(n); if (! _p) return ENOMEM; _p; }) +/* Return malloced storage for N uids. If the malloc fails, return ENOMEM + from the function enclosing the call. */ +#define MALLOC_IDS(n) \ + ({ void *_p = malloc ((n) * sizeof (uid_t)); if (! _p) return ENOMEM; _p; }) - if (noldauxids == 0) - { - *auxids = MALLOC (*nauxids = 1); - (*auxids)[0] = oldgenids[0]; - } - else if (noldauxids == 1) - { - *auxids = MALLOC (*nauxids = 2); - (*auxids)[0] = oldauxids[0]; - (*auxids)[1] = oldgenids[0]; - } - else if (noldauxids == 2) +/* Copy N uids/gids from SRC to DST. */ +#define COPY_IDS(src, dst, n) \ + bcopy (src, dst, (n) * sizeof (uid_t)) + + if (setid) { - if (oldgenids[0] == oldauxids[1]) + /* We are dumping the current first id; put it into the auxids array. + This is complex. The different cases below are intended to make + sure that we don't lose any ids (unlike posix) and to make sure that + aux ids zero and one (if already set) behave like the posix ones. */ + if (noldauxids == 0) { - *auxids = MALLOC (*nauxids = 2); - (*auxids)[0] = oldauxids[0]; - (*auxids)[1] = oldauxids[1]; + if (noldgenids == 0) + _auxids = _nauxids = 0; + else + { + _auxids = MALLOC_IDS (_nauxids = 1); + _auxids[0] = oldgenids[0]; + } } - else + else if (noldauxids == 1) { - /* Shift by one */ - *auxids = MALLOC (*nauxids = 3); - (*auxids)[0] = oldauxids[0]; - (*auxids)[1] = oldgenids[0]; - (*auxids)[2] = oldauxids[1]; + _nauxids = noldgenids > 0 ? 2 : 1; + _auxids = MALLOC_IDS (_nauxids); + _auxids[0] = oldauxids[0]; + if (noldgenids > 0) + _auxids[1] = oldgenids[0]; } - } - else - { - /* Just like above, but in the shift case note - that the new (*auxids)[2] shouldn't be allowed - to needlessly duplicate something further on. */ - if (oldgenids[0] == oldauxids[1] - || scan_ids (&oldauxids[2], *nauxids - 2, oldauxids[1])) + else if (noldauxids == 2) { - *auxids = MALLOC (*nauxids = noldauxids); - bcopy (oldauxids, *auxids, *nauxids); - (*auxids)[1] = oldgenids[0]; + if (noldgenids == 0 || oldgenids[0] == oldauxids[1]) + { + _auxids = MALLOC_IDS (_nauxids = 2); + _auxids[0] = oldauxids[0]; + _auxids[1] = oldauxids[1]; + } + else + { + /* Shift by one */ + _auxids = MALLOC_IDS (_nauxids = 3); + _auxids[0] = oldauxids[0]; + _auxids[1] = oldgenids[0]; + _auxids[2] = oldauxids[1]; + } } - else + else { - *auxids = MALLOC (*nauxids = noldauxids + 1); - (*auxids)[0] = oldauxids[0]; - (*auxids)[1] = oldgenids[0]; - bcopy (&oldauxids[1], &(*auxids)[2], noldauxids - 1); + /* Just like above, but in the shift case note + that the new _auxids[2] shouldn't be allowed + to needlessly duplicate something further on. */ + if (noldgenids == 0 + || oldgenids[0] == oldauxids[1] + || scan_ids (&oldauxids[2], noldauxids - 2, oldauxids[1])) + { + _auxids = MALLOC_IDS (_nauxids = noldauxids); + COPY_IDS (oldauxids, _auxids, _nauxids); + if (noldgenids > 0) + _auxids[1] = oldgenids[0]; + } + else + { + _auxids = MALLOC_IDS (_nauxids = noldauxids + 1); + _auxids[0] = oldauxids[0]; + _auxids[1] = oldgenids[0]; + COPY_IDS (&oldauxids[1], &_auxids[2], noldauxids - 1); + } } - } - /* Whew. Now set the new id. */ - *ngenids = noldgenids ?: 1; - *genids = malloc (*ngenids); + /* Whew. Now set the new id. */ + _ngenids = noldgenids ?: 1; + _genids = malloc (_ngenids * sizeof (uid_t)); - if (! *genids) - { - free (*auxids); - return ENOMEM; + if (! _genids) + { + free (_auxids); + return ENOMEM; + } + + _genids[0] = id; + if (noldgenids > 1) + COPY_IDS (&oldgenids[1], &_genids[1], _ngenids - 1); + + if (secure && !*secure + && !scan_ids (oldgenids, noldgenids, id) + && !scan_ids (oldauxids, noldauxids, id)) + *secure = 1; } + else + /* Not SETID; just copy the old values. */ + { + _ngenids = noldgenids; + _nauxids = noldauxids; + _genids = MALLOC_IDS (_ngenids); + _auxids = malloc (_nauxids * sizeof (uid_t)); - (*genids)[0] = id; - if (noldgenids > 0) - bcopy (&oldgenids[1], &(*genids)[1], *ngenids - 1); + if (! _auxids) + { + free (_genids); + return ENOMEM; + } - if (secure && !*secure - && !scan_ids (oldgenids, noldgenids, id) - && !scan_ids (oldauxids, noldauxids, id)) - *secure = 1; + COPY_IDS (oldgenids, _genids, _ngenids); + COPY_IDS (oldauxids, _auxids, _nauxids); + } + + /* Finally we can zot the return params. */ + *genids = _genids; + *ngenids = _ngenids; + *auxids = _auxids; + *nauxids = _nauxids; return 0; } @@ -199,10 +244,10 @@ diskfs_S_file_exec (struct protid *cred, int noldauxgids = 20, noldgengids = 20; /* These describe the auth port we are trying to create. */ - uid_t *auxuids, *genuids; - gid_t *auxgids, *gengids; - int nauxuids, ngenuids; - int nauxgids, ngengids; + uid_t *auxuids = 0, *genuids = 0; + gid_t *auxgids = 0, *gengids = 0; + int nauxuids = 0, ngenuids = 0; + int nauxgids = 0, ngengids = 0; auth_t newauth; int i; @@ -261,15 +306,14 @@ diskfs_S_file_exec (struct protid *cred, will be wrong. No matter; we will repeat these checks using secure id sets later if the port turns out to be bogus. */ - if (suid) - err = setid (np->dn_stat.st_uid, &secure, - oldgenuids, noldauxuids, oldauxuids, noldauxuids, - &genuids, &ngenuids, &auxuids, &nauxuids); - if (sgid && !err) - err = setid (np->dn_stat.st_gid, &secure, - oldgengids, noldauxgids, oldauxgids, noldauxgids, - &gengids, &ngengids, &auxgids, &nauxgids); + err = setid (suid, np->dn_stat.st_uid, &secure, + oldgenuids, noldgenuids, oldauxuids, noldauxuids, + &genuids, &ngenuids, &auxuids, &nauxuids); + if (! err) + err = setid (sgid, np->dn_stat.st_gid, &secure, + oldgengids, noldgengids, oldauxgids, noldauxgids, + &gengids, &ngengids, &auxgids, &nauxgids); if (scan_ids (oldgenuids, noldgenuids, 0) || scan_ids (oldauxuids, noldauxuids, 0)) secure = 0; /* If we're root, we don't have to be. */ @@ -289,7 +333,7 @@ diskfs_S_file_exec (struct protid *cred, noldauxgids * (sizeof (gid_t))); if (err) - goto abandon_suid; /* setid() failed. */ + goto free_abandon_suid; /* STEP 3: Attempt to create this new auth handle. */ @@ -306,21 +350,19 @@ diskfs_S_file_exec (struct protid *cred, execed (which we know is good), as the effective ids, and assume no aux ids. */ { - /* Free our first attempts. */ - free (genuids); - free (auxuids); - free (gengids); - free (auxgids); - - if (suid) - err = setid (np->dn_stat.st_uid, &secure, - cred->uids, cred->nuids, 0, 0, - &genuids, &ngenuids, &auxuids, &nauxuids); - if (sgid && !err) - err = setid (np->dn_stat.st_gid, &secure, + /* Free our first attempts. [Reinit to 0, so we can free them] */ + free (genuids); genuids = 0; + free (auxuids); auxuids = 0; + free (gengids); gengids = 0; + free (auxgids); auxgids = 0; + + err = setid (suid, np->dn_stat.st_uid, &secure, + cred->uids, cred->nuids, 0, 0, + &genuids, &ngenuids, &auxuids, &nauxuids); + if (! err) + err = setid (sgid, np->dn_stat.st_gid, &secure, cred->gids, cred->ngids, 0, 0, &gengids, &ngengids, &auxgids, &nauxgids); - if (diskfs_isuid (0, cred)) secure = 0; /* If we're root, we don't have to be. */ -- cgit v1.2.3