Download raw body.
got_privsep_recv_tree() fixes
A couple of fixes for got_privsep_recv_tree(). The most important fix is at the bottom: We failed to check whether the number of tree entries sent by the libexec helper exceeds the number of tree entries advertised in the first got_imsg_tree_object message. This makes it trivial for the libexec process to make the main process write beyond the end of its entries array (heap overflow). Besides this, rework the logic to avoid an ugly 'goto', check whether the number of entries is >= 0 (this is an int, which gets converted to size_t when passed to calloc, where it would become a very large number if negative and likely trigger ENOMEM), and free *tree if we fail to allocate (*tree)->entries. Freeing *tree via got_object_tree_close() below depends on (*tree)->nentries being set up properly, which hasn't happened at this point. I should probably make separate commits for each fix but don't want to bother the mailing list with 4 tiny patches. ok? diff 50fa49718233a5cfbfc5160b8dd184d7b203ad3a 8e3ba862ea151c560f30f80551b19608c85138f7 blob - b8e60e5a73e4d935fbc621706294df4e664a14d3 blob + 95397e68aa496b96e22f4b0f1f95a15718b2744d --- lib/privsep.c +++ lib/privsep.c @@ -1566,7 +1566,7 @@ got_privsep_recv_tree(struct got_tree_object **tree, s int nentries = 0; *tree = NULL; -get_more: + err = read_imsg(ibuf); if (err) goto done; @@ -1580,9 +1580,18 @@ get_more: n = imsg_get(ibuf, &imsg); if (n == 0) { - if (*tree && (*tree)->nentries != nentries) - goto get_more; - break; + if ((*tree)) { + if ((*tree)->nentries < nentries) { + err = read_imsg(ibuf); + if (err) + break; + continue; + } else + break; + } else { + err = got_error(GOT_ERR_PRIVSEP_MSG); + break; + } } if (imsg.hdr.len < IMSG_HEADER_SIZE + min_datalen) { @@ -1608,6 +1617,10 @@ get_more: break; } itree = imsg.data; + if (itree->nentries <= 0) { + err = got_error(GOT_ERR_PRIVSEP_LEN); + break; + } *tree = malloc(sizeof(**tree)); if (*tree == NULL) { err = got_error_from_errno("malloc"); @@ -1617,6 +1630,8 @@ get_more: sizeof(struct got_tree_entry)); if ((*tree)->entries == NULL) { err = got_error_from_errno("malloc"); + free(*tree); + *tree = NULL; break; } (*tree)->nentries = itree->nentries; @@ -1645,6 +1660,10 @@ get_more: err = got_error(GOT_ERR_NO_SPACE); break; } + if (nentries >= (*tree)->nentries) { + err = got_error(GOT_ERR_PRIVSEP_LEN); + break; + } te = &(*tree)->entries[nentries]; memcpy(te->name, imsg.data + sizeof(*ite), datalen); te->name[datalen] = '\0';
got_privsep_recv_tree() fixes