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

[PATCH] Fix memory corruption when .gitignore does not end by \n

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Jan 20, 2010, 14:09 UTC
Message-ID
<1263996556-9712-1-git-send-email-pclouds@gmail.com>
In-Reply-To
<fcaeb9bf1001200458v436a8adeq5cfc6753900e6c0c@mail.gmail.com>

Commit b5041c5 (Avoid writing to buffer in add_excludes_from_file_1()) tried not to append '\n' at the end because the next commit may return a buffer that does not have extra space for that.

Unfortunately it left this assignment in the loop:
  buf[i - (i && buf[i-1] == '\r')] = 0;

that can corrupt memory if "buf" is not '\n' terminated. But even if it does not corrupt memory, the last line would not be NULL-terminated, leading to errors later inside add_exclude().

This patch fixes it by reverting the faulty commit and make sure "buf" is always \n terminated.

While at it, free unused memory properly.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 This patch causes a crash for me. Not sure if it does for anybody else.
  diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
  index 6d2f2b6..e7efdb5 100755
  --- a/t/t3001-ls-files-others-exclude.sh
  +++ b/t/t3001-ls-files-others-exclude.sh
  @@ -57,7 +57,7 @@ expect
   echo '*.1
   /*.3
   !*.6' >.gitignore
  -echo '*.2
  +echo -n '*.2
   two/*.4
   !*.7
   *.8' >one/.gitignore
 dir.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/dir.c b/dir.c
index 1538ad5..67c3af6 100644
--- a/dir.c
+++ b/dir.c
@@ -242,6 +242,14 @@ int add_excludes_from_file_to_list(const char *fname,
 		if (!check_index ||
 		    (buf = read_skip_worktree_file_from_index(fname, &size)) == NULL)
 			return -1;
+		if (size == 0) {
+			free(buf);
+			return 0;
+		}
+		if (buf[size-1] != '\n') {
+			buf = xrealloc(buf, size+1);
+			buf[size++] = '\n';
+		}
 	}
 	else {
 		size = xsize_t(st.st_size);
@@ -249,19 +257,21 @@ int add_excludes_from_file_to_list(const char *fname,
 			close(fd);
 			return 0;
 		}
-		buf = xmalloc(size);
+		buf = xmalloc(size+1);
 		if (read_in_full(fd, buf, size) != size) {
+			free(buf);
 			close(fd);
 			return -1;
 		}
+		buf[size++] = '\n';
 		close(fd);
 	}
 
 	if (buf_p)
 		*buf_p = buf;
 	entry = buf;
-	for (i = 0; i <= size; i++) {
-		if (i == size || buf[i] == '\n') {
+	for (i = 0; i < size; i++) {
+		if (buf[i] == '\n') {
 			if (entry != buf + i && entry[0] != '#') {
 				buf[i - (i && buf[i-1] == '\r')] = 0;
 				add_exclude(entry, base, baselen, which);
-- 
1.6.6.181.g5ee6
Previous: Nguyen Thai Ngoc DuyNext: Junio C Hamano
Message 6 of 10 in “git-status segmentation fault in master / OS X”
  1. Jonathan del StrotherJan 19, 2010
  2. Jeff KingJan 20, 2010
  3. Junio C HamanoJan 20, 2010
  4. Jonathan del StrotherJan 20, 2010
  5. Nguyen Thai Ngoc DuyJan 20, 2010
  6. Fix memory corruption when .gitignore does not end by \nNguyễn Thái Ngọc Duy, Jan 20, 2010
  7. Junio C HamanoJan 20, 2010
  8. Nguyen Thai Ngoc DuyJan 21, 2010
  9. Jonathan del StrotherJan 21, 2010
  10. Junio C HamanoJan 21, 2010

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.