git: 3736b2dd3270 - main - diff: Fix a use after free as well as a memory leak in change().

From: John Baldwin <jhb_at_FreeBSD.org>
Date: Mon, 03 Oct 2022 23:11:30 UTC
The branch main has been updated by jhb:

URL: https://cgit.FreeBSD.org/src/commit/?id=3736b2dd327050d2e6c925964b210eccbaac51ab

commit 3736b2dd327050d2e6c925964b210eccbaac51ab
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2022-10-03 23:10:43 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2022-10-03 23:10:43 +0000

    diff: Fix a use after free as well as a memory leak in change().
    
    When -B or -I are used, change() evaluates the lines in a hunk to
    determine if it is a hunk that should be ignored.  It does this by
    reading each candidate line into a mallocated buffer via preadline()
    and then calling ignoreline().  Previously the buffer was freed as a
    side effect of ignoreline_pattern() called from ignoreline().
    However, if only -B was specified, then ignoreline_pattern() was not
    called and the lines were leaked.  If both options were specified,
    then ignoreline_pattern() was called before checking for a blank line
    so that the second check was a use after free.
    
    To fix, pull the free() out of ignoreline_pattern() and instead do it
    up in change() so that is paired with preadline().
    
    While here, simplify ignoreline() by checking for the -B and -I cases
    individually without a separate clause for when both are set.  Also,
    do the cheaper check (-B) first, and remove a false comment (this
    function is only called if at least one of -I or -B are specified).
    
    Reviewed by:    emaste
    Reported by:    GCC 12 -Wuse-after-free
    Differential Revision:  https://reviews.freebsd.org/D36822
---
 usr.bin/diff/diffreg.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/usr.bin/diff/diffreg.c b/usr.bin/diff/diffreg.c
index 4fbdd3c3f50c..c9111fe7eaee 100644
--- a/usr.bin/diff/diffreg.c
+++ b/usr.bin/diff/diffreg.c
@@ -980,7 +980,6 @@ ignoreline_pattern(char *line)
 	int ret;
 
 	ret = regexec(&ignore_re, line, 0, NULL, 0);
-	free(line);
 	return (ret == 0);	/* if it matched, it should be ignored. */
 }
 
@@ -988,13 +987,10 @@ static bool
 ignoreline(char *line, bool skip_blanks)
 {
 
-	if (ignore_pats != NULL && skip_blanks)
-		return (ignoreline_pattern(line) || *line == '\0');
-	if (ignore_pats != NULL)
-		return (ignoreline_pattern(line));
-	if (skip_blanks)
-		return (*line == '\0');
-	/* No ignore criteria specified */
+	if (skip_blanks && *line == '\0')
+		return (true);
+	if (ignore_pats != NULL && ignoreline_pattern(line))
+		return (true);
 	return (false);
 }
 
@@ -1013,7 +1009,7 @@ change(char *file1, FILE *f1, char *file2, FILE *f2, int a, int b, int c, int d,
 	long curpos;
 	int i, nc;
 	const char *walk;
-	bool skip_blanks;
+	bool skip_blanks, ignore;
 
 	skip_blanks = (*pflags & D_SKIPBLANKLINES);
 restart:
@@ -1030,7 +1026,9 @@ restart:
 			for (i = a; i <= b; i++) {
 				line = preadline(fileno(f1),
 				    ixold[i] - ixold[i - 1], ixold[i - 1]);
-				if (!ignoreline(line, skip_blanks))
+				ignore = ignoreline(line, skip_blanks);
+				free(line);
+				if (!ignore)
 					goto proceed;
 			}
 		}
@@ -1038,7 +1036,9 @@ restart:
 			for (i = c; i <= d; i++) {
 				line = preadline(fileno(f2),
 				    ixnew[i] - ixnew[i - 1], ixnew[i - 1]);
-				if (!ignoreline(line, skip_blanks))
+				ignore = ignoreline(line, skip_blanks);
+				free(line);
+				if (!ignore)
 					goto proceed;
 			}
 		}