threads / patch / 6982

patchdiff --cc: integer overflow given a 2GB-or-larger file

Subject: [PATCH] diff --cc: integer overflow given a 2GB-or-larger file

## tl;dr

One message between Feb 26, 2007 and Feb 26, 2007. Diffs are folded; open one to read it.

replies: 0people: 1as markdown or json

Jim Meyering· Feb 26, 2007, 23:11 UTC · lore

Few of us use git to compare or even version-control 2GB files, but when we do, we'll want it to work.

Reading a recent patch, I noticed two lines like this:
   int len = st.st_size;

Instead of "int", that should be "size_t". Otherwise, in the non-symlink case, with 64-bit size_t, if the file's size is 2GB, the following xmalloc will fail:

   result = xmalloc(len + 1);

trying to allocate 2^64 - 2^31 + 1 bytes (assuming sign-extension in the int-to-size_t promotion). And even if it didn't fail, the subsequent "result[len] = 0;" would be equivalent to an unpleasant "result[-2147483648] = 0;"

The other nearby "int"-declared size variable, sz, should also be of type size_t, for the same reason. If sz ever wraps around and becomes negative, xread will corrupt memory _before_ the "result" buffer.

Signed-off-by: Jim Meyering <jim@meyering.net>
---
 combine-diff.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
Show changes to combine-diff.c +3 −3
diff --git a/combine-diff.c b/combine-diff.c
index 6b7c6be..044633d 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -684,7 +684,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			goto deleted_file;
 
 		if (S_ISLNK(st.st_mode)) {
-			int len = st.st_size;
+			size_t len = st.st_size;
 			result_size = len;
 			result = xmalloc(len + 1);
 			if (result_size != readlink(elem->path, result, len)) {
@@ -697,8 +697,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 		}
 		else if (0 <= (fd = open(elem->path, O_RDONLY)) &&
 			 !fstat(fd, &st)) {
-			int len = st.st_size;
-			int sz = 0;
+			size_t len = st.st_size;
+			size_t sz = 0;
 
 			elem->mode = canon_mode(st.st_mode);
 			result_size = len;
-- 
1.5.0.1.226.g7bd59

← back to recent threads