commit 0d039490032e7afdddb00a59c0e6240c9b1a4dfa
from: Mark Jamsek <mark@jamsek.dev>
via: Thomas Adam <thomas@xteddy.org>
date: Mon Jun 03 19:10:22 2024 UTC

plug 'got diff obj1 obj2' line metadata memory leak

Despite API callers not requesting it, we collect line metadata due to NULL
pointer checks missing a level of indirection.  Reported by Kyle Ackerman.

ok op and stsp

commit - 0f24033e38cc689c7c86333f00269a799660dd38
commit + 0d039490032e7afdddb00a59c0e6240c9b1a4dfa
blob - aa67f591fb351d377bca13b28e1dc0b7296e8a96
blob + 50e27eac588c197ea4e8e49c301c37a2bb14da90
--- lib/diff.c
+++ lib/diff.c
@@ -146,12 +146,15 @@ diff_blobs(struct got_diff_line **lines, size_t *nline
 	off_t outoff = 0;
 	int n;
 
-	if (lines && *lines && *nlines > 0)
-		outoff = (*lines)[*nlines - 1].offset;
-	else if (lines) {
-		err = add_line_metadata(lines, nlines, 0, GOT_DIFF_LINE_NONE);
-		if (err)
-			goto done;
+	if (lines && *lines) {
+		if (*nlines > 0)
+			outoff = (*lines)[*nlines - 1].offset;
+		else {
+			err = add_line_metadata(lines, nlines,
+			    0, GOT_DIFF_LINE_NONE);
+			if (err != NULL)
+				goto done;
+		}
 	}
 
 	if (resultp)
@@ -218,7 +221,7 @@ diff_blobs(struct got_diff_line **lines, size_t *nline
 		if (n < 0)
 			goto done;
 		outoff += n;
-		if (lines) {
+		if (lines && *lines) {
 			err = add_line_metadata(lines, nlines, outoff,
 			    GOT_DIFF_LINE_BLOB_MIN);
 			if (err)
@@ -230,7 +233,7 @@ diff_blobs(struct got_diff_line **lines, size_t *nline
 		if (n < 0)
 			goto done;
 		outoff += n;
-		if (lines) {
+		if (lines && *lines) {
 			err = add_line_metadata(lines, nlines, outoff,
 			    GOT_DIFF_LINE_BLOB_PLUS);
 			if (err)
blob - 0c5437a04f86df6eb79d9ec6ceb9a8f373d2704f
blob + 36e65fe9dbf6828fbe4915d554b475498d2bd900
--- lib/diffreg.c
+++ lib/diffreg.c
@@ -272,13 +272,13 @@ got_diffreg_output(struct got_diff_line **lines, size_
 	switch (output_format) {
 	case GOT_DIFF_OUTPUT_UNIDIFF:
 		rc = diff_output_unidiff(
-		    lines ? &output_info : NULL, outfile, &info,
+		    lines && *lines ? &output_info : NULL, outfile, &info,
 		    diff_result->result, context_lines);
 		if (rc != DIFF_RC_OK)
 			return got_error_set_errno(rc, "diff_output_unidiff");
 		break;
 	case GOT_DIFF_OUTPUT_PLAIN:
-		rc = diff_output_plain(lines ? &output_info : NULL,
+		rc = diff_output_plain(lines && *lines ? &output_info : NULL,
 		    outfile, &info, diff_result->result, 1);
 		if (rc != DIFF_RC_OK)
 			return got_error_set_errno(rc, "diff_output_edscript");