commit 95326260eb5a3786dbd04e735b6c8c9313523ff2
from: Omar Polo <op@omarpolo.com>
via: Thomas Adam <thomas@xteddy.org>
date: Mon Sep 05 13:07:23 2022 UTC

gotwebd: fix usage of the commit graph iter

got_get_repo_commits mixes ids allocated on the heap and the one
returned by the iter_next in the same variable, and then even attemp to
free it.

This is both a leak (we loose the pointer to the previously allocated
id) and a possible invalid free since the object id pointer returned by
the graph iterator is not to be passed to free(3).

part of a bigger diff that's ok stsp@

commit - 7ee8c11a1c0b35faced7b53a32b5a43b0f45b764
commit + 95326260eb5a3786dbd04e735b6c8c9313523ff2
blob - 1e1c4c048bd8bcfcf5316731a99f142af04a4d72
blob + 7aae170c238664c91f6d635261100cb7454437c5
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -434,6 +434,8 @@ got_get_repo_commits(struct request *c, int limit)
 		goto done;
 
 	for (;;) {
+		struct got_object_id *next_id;
+
 		if (limit_chk == ((limit * qs->page) - (limit - 1)) &&
 		    commit_found == 0 && repo_commit &&
 		    repo_commit->commit_id != NULL) {
@@ -444,17 +446,15 @@ got_get_repo_commits(struct request *c, int limit)
 			}
 		}
 
-		error = got_commit_graph_iter_next(&id, graph, repo, NULL,
+		error = got_commit_graph_iter_next(&next_id, graph, repo, NULL,
 		    NULL);
 		if (error) {
 			if (error->code == GOT_ERR_ITER_COMPLETED)
 				error = NULL;
-			goto done;
-		}
-		if (id == NULL)
 			goto done;
+		}
 
-		error = got_object_open_as_commit(&commit, repo, id);
+		error = got_object_open_as_commit(&commit, repo, next_id);
 		if (error)
 			goto done;
 
@@ -470,7 +470,7 @@ got_get_repo_commits(struct request *c, int limit)
 		TAILQ_INSERT_TAIL(&t->repo_commits, repo_commit, entry);
 
 		error = got_get_repo_commit(c, repo_commit, commit,
-		    &refs, id);
+		    &refs, next_id);
 		if (error)
 			goto done;
 
@@ -482,15 +482,10 @@ got_get_repo_commits(struct request *c, int limit)
 				commit_found = 1;
 			else {
 				limit_chk++;
-				free(id);
-				id = NULL;
 				continue;
 			}
 		}
 
-		free(id);
-		id = NULL;
-
 		if (limit == 1 && chk_multi == 0 &&
 		    srv->max_commits_display != 1)
 			commit_found = 1;