commit 5afebbd1b6bcfecf69d25e367e95c9906245fcf5
from: Stefan Sperling <stsp@stsp.name>
via: Mark Jamsek <mark@jamsek.dev>
date: Wed Feb 22 10:21:53 2023 UTC

fix tog diff between arbitrary commits

Don't assume commit info is always written. This is only true when diffing a
commit against (one of) its direct parent(s). Otherwise we perform an invalid
read on a tmp got_diff_line array and end up passing bogus offsets to fseeko().
Bug found and fixed by stsp with a minor tweak by me.

ok jamsek for stsp's initial diff
ok stsp@

commit - fd8d60a2d11af314daec9c6c7ad0ea5c7ac0abd0
commit + 5afebbd1b6bcfecf69d25e367e95c9906245fcf5
blob - 0d706f1c0a5161fd7a132b213b89c64e4cd17da0
blob + 684d848b2cfddf5e676a0f04df5a276de4136861
--- tog/tog.c
+++ tog/tog.c
@@ -4578,14 +4578,23 @@ cat_diff(FILE *dst, FILE *src, struct got_diff_line **
 			return got_ferror(dst, GOT_ERR_IO);
 	}
 
+	if (s_nlines == 0 && *d_nlines == 0)
+		return NULL;
+
 	/*
-	 * The diff driver initialises the first line at offset zero when the
-	 * array isn't prepopulated, skip it; we already have it in *d_lines.
+	 * If commit info was in dst, increment line offsets
+	 * of the appended diff content, but skip s_lines[0]
+	 * because offset zero is already in *d_lines.
 	 */
-	for (i = 1; i < s_nlines; ++i)
-		s_lines[i].offset += (*d_lines)[*d_nlines - 1].offset;
+	if (*d_nlines > 0) {
+		for (i = 1; i < s_nlines; ++i)
+			s_lines[i].offset += (*d_lines)[*d_nlines - 1].offset;
 
-	--s_nlines;
+		if (s_nlines > 0) {
+			--s_nlines;
+			++s_lines;
+		}
+	}
 
 	p = reallocarray(*d_lines, *d_nlines + s_nlines, sizeof(*p));
 	if (p == NULL) {
@@ -4595,7 +4604,7 @@ cat_diff(FILE *dst, FILE *src, struct got_diff_line **
 
 	*d_lines = p;
 
-	memcpy(*d_lines + *d_nlines, s_lines + 1, s_nlines * sizeof(*s_lines));
+	memcpy(*d_lines + *d_nlines, s_lines, s_nlines * sizeof(*s_lines));
 	*d_nlines += s_nlines;
 
 	return NULL;