commit 47cb6f8b62ed4ad39838a5d4702f593b23d3e7f2
from: Omar Polo <op@omarpolo.com>
date: Sun Feb 26 21:34:11 2023 UTC

fix GOT_IMSG_COMMIT_TRAVERSAL_REQUEST

The sending and receiving side are sending the data differently.  It
works now by chance since a struct got_object_id is "just" a
SHA1_DIGEST_LENGTH sized buffer, but will break in the future.
Furthermore, the structure of the data as described in got_lib_privsep.h
is not respected, as the path_len field is not transmitted.

Change it to send/receive a whole struct got_object_id, the path without
the NUL and the proper length.  (The path is also actually mandatory, so
assert it too.)

improvements + ok stsp@

commit - d6baa0f6da2663235688f774619dbf494add2f27
commit + 47cb6f8b62ed4ad39838a5d4702f593b23d3e7f2
blob - 7599c74782932b66b186f9bb91980bd32cd5c663
blob + d517f36f2ccbda4c48348016a20e6d6e87fbd1a4
--- lib/got_lib_privsep.h
+++ lib/got_lib_privsep.h
@@ -576,8 +576,7 @@ struct got_imsg_object_idlist {
 
 /* Structure for GOT_IMSG_COMMIT_TRAVERSAL_REQUEST  */
 struct got_imsg_commit_traversal_request {
-	uint8_t id[SHA1_DIGEST_LENGTH];
-	int idx;
+	struct got_imsg_packed_object iobj;
 	size_t path_len;
 	/* Followed by path_len bytes of path data */
 } __attribute__((__packed__));
blob - b8ea370fd2e88931bfc3f441dded6c848d55b3bd
blob + cfa0e0b64748e7fbb205cd975b92dc071f1a7510
--- lib/privsep.c
+++ lib/privsep.c
@@ -2650,19 +2650,26 @@ got_privsep_send_commit_traversal_request(struct imsgb
     struct got_object_id *id, int idx, const char *path)
 {
 	struct ibuf *wbuf;
-	size_t path_len = strlen(path) + 1;
+	size_t path_len = strlen(path);
 
 	wbuf = imsg_create(ibuf, GOT_IMSG_COMMIT_TRAVERSAL_REQUEST, 0, 0,
 	    sizeof(struct got_imsg_commit_traversal_request) + path_len);
 	if (wbuf == NULL)
 		return got_error_from_errno(
 		    "imsg_create COMMIT_TRAVERSAL_REQUEST");
-	if (imsg_add(wbuf, id->sha1, SHA1_DIGEST_LENGTH) == -1)
+	/*
+	 * Keep in sync with struct got_imsg_commit_traversal_request
+	 * and struct got_imsg_packed_object.
+	 */
+	if (imsg_add(wbuf, id, sizeof(*id)) == -1)
 		return got_error_from_errno("imsg_add "
 		    "COMMIT_TRAVERSAL_REQUEST");
 	if (imsg_add(wbuf, &idx, sizeof(idx)) == -1)
 		return got_error_from_errno("imsg_add "
 		    "COMMIT_TRAVERSAL_REQUEST");
+	if (imsg_add(wbuf, &path_len, sizeof(path_len)) == -1)
+		return got_error_from_errno("imsg_add "
+		    "COMMIT_TRAVERSAL_REQUEST");
 	if (imsg_add(wbuf, path, path_len) == -1)
 		return got_error_from_errno("imsg_add "
 		    "COMMIT_TRAVERSAL_REQUEST");
blob - ed4f9039ad8c4841bba30c73ac1ec2a413e881e7
blob + 62e4cf4d156d8e4fbbc26a8f1d65893a5da67f2b
--- libexec/got-read-pack/got-read-pack.c
+++ libexec/got-read-pack/got-read-pack.c
@@ -602,34 +602,34 @@ commit_traversal_request(struct imsg *imsg, struct ims
     struct got_object_cache *objcache)
 {
 	const struct got_error *err = NULL;
-	struct got_imsg_packed_object iobj;
+	struct got_imsg_commit_traversal_request ctreq;
 	struct got_object_qid *pid;
 	struct got_commit_object *commit = NULL, *pcommit = NULL;
 	struct got_parsed_tree_entry *entries = NULL, *pentries = NULL;
 	size_t nentries = 0, nentries_alloc = 0;
 	size_t pnentries = 0, pnentries_alloc = 0;
 	struct got_object_id id;
-	size_t datalen, path_len;
+	size_t datalen;
 	char *path = NULL;
 	const int min_alloc = 64;
 	int changed = 0, ncommits = 0, nallocated = 0;
 	struct got_object_id *commit_ids = NULL;
 
 	datalen = imsg->hdr.len - IMSG_HEADER_SIZE;
-	if (datalen < sizeof(iobj))
-		return got_error(GOT_ERR_PRIVSEP_LEN);
-	memcpy(&iobj, imsg->data, sizeof(iobj));
-	memcpy(&id, &iobj.id, sizeof(id));
+	if (datalen < sizeof(ctreq))
+		return got_error(GOT_ERR_PRIVSEP_LEN);
+	memcpy(&ctreq, imsg->data, sizeof(ctreq));
+	memcpy(&id, &ctreq.iobj.id, sizeof(id));
 
-	path_len = datalen - sizeof(iobj) - 1;
-	if (path_len < 0)
+	if (datalen != sizeof(ctreq) + ctreq.path_len)
 		return got_error(GOT_ERR_PRIVSEP_LEN);
-	if (path_len > 0) {
-		path = imsg->data + sizeof(iobj);
-		if (path[path_len] != '\0')
-			return got_error(GOT_ERR_PRIVSEP_LEN);
-	}
+	if (ctreq.path_len == 0)
+		return got_error(GOT_ERR_PRIVSEP_LEN);
 
+	path = strndup(imsg->data + sizeof(ctreq), ctreq.path_len);
+	if (path == NULL)
+		return got_error_from_errno("strndup");
+
 	nallocated = min_alloc;
 	commit_ids = reallocarray(NULL, nallocated, sizeof(*commit_ids));
 	if (commit_ids == NULL)
@@ -767,6 +767,7 @@ commit_traversal_request(struct imsg *imsg, struct ims
 	}
 	err = send_commit_traversal_done(ibuf);
 done:
+	free(path);
 	free(commit_ids);
 	if (commit)
 		got_object_commit_close(commit);