Skip to content

Commit 831a683

Browse files
committed
cvtsudoers_make_grlist_item: fix heap overflow on reallocation
cvtsudoers_make_grlist_item() estimated the size of the buffer used to store a group list item based on the system's maximum group length. However, it is possible for the invoking user to specify a group of arbitrary length. There is a check to avoid overflowing the buffer if the estimate is too small, but that code path re-uses the ngroups variable which has since been reset. This can result in the resized buffer being too small, leading to an overflow. Since we have the list of groups to be added, we can allocate the needed amount instead of estimating it. This removes the need for reallocation on overflow. Reported by Bartlomiej Dmitruk
1 parent b890d52 commit 831a683

1 file changed

Lines changed: 8 additions & 10 deletions

File tree

plugins/sudoers/cvtsudoers_pwutil.c

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,6 @@ cvtsudoers_make_grlist_item(const struct passwd *pw, char * const *unused1)
390390
struct cache_item_grlist *grlitem;
391391
struct sudoers_string *s;
392392
struct group_list *grlist;
393-
const size_t groupname_len = sudo_login_name_max();
394393
debug_decl(cvtsudoers_make_grlist_item, SUDOERS_DEBUG_NSS);
395394

396395
/*
@@ -401,18 +400,17 @@ cvtsudoers_make_grlist_item(const struct passwd *pw, char * const *unused1)
401400
debug_return_ptr(&grlist_item->cache);
402401
}
403402

404-
/* Count number of groups in the filter. */
403+
/* Allocate in one big chunk for easy freeing. */
404+
nsize = strlen(pw->pw_name) + 1;
405+
total = sizeof(*grlitem) + nsize;
406+
407+
/* Count groups in the filter and add space for each one. */
405408
ngroups = 0;
406409
STAILQ_FOREACH(s, &filters->groups, entries) {
410+
total += sizeof(char *) + strlen(s->str) + 1;
407411
ngroups++;
408412
}
409413

410-
/* Allocate in one big chunk for easy freeing. */
411-
nsize = strlen(pw->pw_name) + 1;
412-
total = sizeof(*grlitem) + nsize;
413-
total += groupname_len * ngroups;
414-
415-
again:
416414
if ((grlitem = calloc(1, total)) == NULL) {
417415
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO,
418416
"unable to allocate memory");
@@ -451,9 +449,9 @@ cvtsudoers_make_grlist_item(const struct passwd *pw, char * const *unused1)
451449
}
452450
len = strlen(s->str) + 1;
453451
if ((size_t)(cp - (char *)grlitem) + len > total) {
454-
total += len + groupname_len;
452+
sudo_warnx(U_("internal error, %s overflow"), __func__);
455453
free(grlitem);
456-
goto again;
454+
debug_return_ptr(NULL);
457455
}
458456
memcpy(cp, s->str, len);
459457
grlist->groups[ngroups++] = cp;

0 commit comments

Comments
 (0)