Skip to content

Commit f1c0e39

Browse files
ttaylorrgitster
authored andcommitted
apply: reject patches larger than ~1 GiB
The apply code is not prepared to handle extremely large files. It uses "int" in some places, and "unsigned long" in others. This combination leads to unfortunate problems when switching between the two types. Using "int" prevents us from handling large files, since large offsets will wrap around and spill into small negative values, which can result in wrong behavior (like accessing the patch buffer with a negative offset). Converting from "unsigned long" to "int" also has truncation problems even on LLP64 platforms where "long" is the same size as "int", since the former is unsigned but the latter is not. To avoid potential overflow and truncation issues in `git apply`, apply similar treatment as in dcd1742 (xdiff: reject files larger than ~1GB, 2015-09-24), where the xdiff code was taught to reject large files for similar reasons. The maximum size was chosen somewhat arbitrarily, but picking a value just shy of a gigabyte allows us to double it without overflowing 2^31-1 (after which point our value would wrap around to a negative number). To give ourselves a bit of extra margin, the maximum patch size is a MiB smaller than a full GiB, which gives us some slop in case we allocate "(records + 1) * sizeof(int)" or similar. Luckily, the security implications of these conversion issues are relatively uninteresting, because a victim needs to be convinced to apply a malicious patch. Reported-by: 정재우 <[email protected]> Suggested-by: Johannes Schindelin <[email protected]> Signed-off-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent d5b4139 commit f1c0e39

File tree

2 files changed

+34
-1
lines changed

2 files changed

+34
-1
lines changed

apply.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,9 +386,19 @@ static void say_patch_name(FILE *output, const char *fmt, struct patch *patch)
386386

387387
#define SLOP (16)
388388

389+
/*
390+
* apply.c isn't equipped to handle arbitrarily large patches, because
391+
* it intermingles `unsigned long` with `int` for the type used to store
392+
* buffer lengths.
393+
*
394+
* Only process patches that are just shy of 1 GiB large in order to
395+
* avoid any truncation or overflow issues.
396+
*/
397+
#define MAX_APPLY_SIZE (1024UL * 1024 * 1023)
398+
389399
static int read_patch_file(struct strbuf *sb, int fd)
390400
{
391-
if (strbuf_read(sb, fd, 0) < 0)
401+
if (strbuf_read(sb, fd, 0) < 0 || sb->len >= MAX_APPLY_SIZE)
392402
return error_errno("git apply: failed to read");
393403

394404
/*

t/t4141-apply-too-large.sh

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#!/bin/sh
2+
3+
test_description='git apply with too-large patch'
4+
5+
TEST_PASSES_SANITIZE_LEAK=true
6+
. ./test-lib.sh
7+
8+
test_expect_success EXPENSIVE 'git apply rejects patches that are too large' '
9+
sz=$((1024 * 1024 * 1023)) &&
10+
{
11+
cat <<-\EOF &&
12+
diff --git a/file b/file
13+
new file mode 100644
14+
--- /dev/null
15+
+++ b/file
16+
@@ -0,0 +1 @@
17+
EOF
18+
test-tool genzeros
19+
} | test_copy_bytes $sz | test_must_fail git apply 2>err &&
20+
grep "git apply: failed to read" err
21+
'
22+
23+
test_done

0 commit comments

Comments
 (0)