git: 3736b2dd3270 - main - diff: Fix a use after free as well as a memory leak in change().
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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;
}
}