git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] apply: avoid out-of-bounds access in fuzzy_matchlines()

From
René Scharfe <l.s.r@web.de>
Date
Nov 11, 2017, 14:10 UTC
Message-ID
<6ff27de7-ac22-3f2f-1f3b-2e0e6f10605a@web.de>
In-Reply-To
<0101015f9c91871f-2f750aec-6877-4e29-9c15-c8399670dd48-000000@us-west-2.amazonses.com>

fuzzy_matchlines() uses a pointers to the first and last characters of two lines to keep track while matching them. This makes it impossible to deal with empty strings. It accesses characters before the start of empty lines. It can also access characters after the end when checking for trailing whitespace in the main loop.

Avoid that by using pointers to the first character and the one *after* the last one. This is well-defined as long as the latter is not dereferenced. Basically rewrite the function based on that premise; it becomes much simpler as a result. There is no need to check for leading whitespace outside of the main loop anymore.

Reported-by: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 apply.c | 59 ++++++++++++++++++++---------------------------------------
 1 file changed, 20 insertions(+), 39 deletions(-)
diff --git a/apply.c b/apply.c
index d676debd59..b8087bd29c 100644
--- a/apply.c
+++ b/apply.c
@@ -300,52 +300,33 @@ static uint32_t hash_line(const char *cp, size_t len)
 static int fuzzy_matchlines(const char *s1, size_t n1,
 			    const char *s2, size_t n2)
 {
-	const char *last1 = s1 + n1 - 1;
-	const char *last2 = s2 + n2 - 1;
-	int result = 0;
+	const char *end1 = s1 + n1;
+	const char *end2 = s2 + n2;
 
 	/* ignore line endings */
-	while ((*last1 == '\r') || (*last1 == '\n'))
-		last1--;
-	while ((*last2 == '\r') || (*last2 == '\n'))
-		last2--;
-
-	/* skip leading whitespaces, if both begin with whitespace */
-	if (s1 <= last1 && s2 <= last2 && isspace(*s1) && isspace(*s2)) {
-		while (isspace(*s1) && (s1 <= last1))
-			s1++;
-		while (isspace(*s2) && (s2 <= last2))
-			s2++;
-	}
-	/* early return if both lines are empty */
-	if ((s1 > last1) && (s2 > last2))
-		return 1;
-	while (!result) {
-		result = *s1++ - *s2++;
-		/*
-		 * Skip whitespace inside. We check for whitespace on
-		 * both buffers because we don't want "a b" to match
-		 * "ab"
-		 */
-		if (isspace(*s1) && isspace(*s2)) {
-			while (isspace(*s1) && s1 <= last1)
+	while (s1 < end1 && (end1[-1] == '\r' || end1[-1] == '\n'))
+		end1--;
+	while (s2 < end2 && (end2[-1] == '\r' || end2[-1] == '\n'))
+		end2--;
+
+	while (s1 < end1 && s2 < end2) {
+		if (isspace(*s1)) {
+			/*
+			 * Skip whitespace. We check on both buffers
+			 * because we don't want "a b" to match "ab".
+			 */
+			if (!isspace(*s2))
+				return 0;
+			while (s1 < end1 && isspace(*s1))
 				s1++;
-			while (isspace(*s2) && s2 <= last2)
+			while (s2 < end2 && isspace(*s2))
 				s2++;
-		}
-		/*
-		 * If we reached the end on one side only,
-		 * lines don't match
-		 */
-		if (
-		    ((s2 > last2) && (s1 <= last1)) ||
-		    ((s1 > last1) && (s2 <= last2)))
+		} else if (*s1++ != *s2++)
 			return 0;
-		if ((s1 > last1) && (s2 > last2))
-			break;
 	}
 
-	return !result;
+	/* If we reached the end on one side only, lines don't match. */
+	return s1 == end1 && s2 == end2;
 }
 
 static void add_line_info(struct image *img, const char *bol, size_t len, unsigned flag)
-- 
2.15.0
Previous: René ScharfeNext: Junio C Hamano
Message 3 of 5 in “Invalid memory access in `git apply`”
  1. mqudsi@neosmart.netNov 8, 2017
  2. René ScharfeNov 11, 2017
  3. apply: avoid out-of-bounds access in fuzzy_matchlines()René Scharfe, Nov 11, 2017
  4. Junio C HamanoNov 12, 2017
  5. apply: update line lengths for --inaccurate-eofRené Scharfe, Nov 16, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.