Commit 54f0a27803c8e64592e7407ef5df0f75a9e2fc0c
2019-10-21T18:56:59
patch_parse: detect overflow when calculating old/new line position When the patch contains lines close to INT_MAX, then it may happen that we end up with an integer overflow when calculating the line of the current diff hunk. Reject such patches as unreasonable to avoid the integer overflow. As the calculation is performed on integers, we introduce two new helpers `git__add_int_overflow` and `git__sub_int_overflow` that perform the integer overflow check in a generic way.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112
diff --git a/src/integer.h b/src/integer.h
index 10b1097..c57681a 100644
--- a/src/integer.h
+++ b/src/integer.h
@@ -65,15 +65,25 @@ GIT_INLINE(int) git__is_int(long long p)
# error compiler has add with overflow intrinsics but SIZE_MAX is unknown
# endif
+# define git__add_int_overflow(out, one, two) \
+ __builtin_sadd_overflow(one, two, out)
+# define git__sub_int_overflow(out, one, two) \
+ __builtin_ssub_overflow(one, two, out)
+
/* Use Microsoft's safe integer handling functions where available */
#elif defined(_MSC_VER)
+# define ENABLE_INTSAFE_SIGNED_FUNCTIONS
# include <intsafe.h>
# define git__add_sizet_overflow(out, one, two) \
(SizeTAdd(one, two, out) != S_OK)
# define git__multiply_sizet_overflow(out, one, two) \
(SizeTMult(one, two, out) != S_OK)
+#define git__add_int_overflow(out, one, two) \
+ (IntAdd(one, two, out) != S_OK)
+#define git__sub_int_overflow(out, one, two) \
+ (IntSub(one, two, out) != S_OK)
#else
@@ -101,6 +111,24 @@ GIT_INLINE(bool) git__multiply_sizet_overflow(size_t *out, size_t one, size_t tw
return false;
}
+GIT_INLINE(bool) git__add_int_overflow(int *out, int one, int two)
+{
+ if ((two > 0 && one > (INT_MAX - two)) ||
+ (two < 0 && one < (INT_MIN - two)))
+ return true;
+ *out = one + two;
+ return false;
+}
+
+GIT_INLINE(bool) git__sub_int_overflow(int *out, int one, int two)
+{
+ if ((two > 0 && one < (INT_MIN + two)) ||
+ (two < 0 && one > (INT_MAX + two)))
+ return true;
+ *out = one - two;
+ return false;
+}
+
#endif
#endif
diff --git a/src/patch_parse.c b/src/patch_parse.c
index 7f39216..87aab96 100644
--- a/src/patch_parse.c
+++ b/src/patch_parse.c
@@ -571,11 +571,17 @@ static int parse_hunk_body(
!git_parse_ctx_contains_s(&ctx->parse_ctx, "@@ -");
git_parse_advance_line(&ctx->parse_ctx)) {
+ int old_lineno, new_lineno, origin, prefix = 1;
char c;
- int origin;
- int prefix = 1;
- int old_lineno = hunk->hunk.old_start + (hunk->hunk.old_lines - oldlines);
- int new_lineno = hunk->hunk.new_start + (hunk->hunk.new_lines - newlines);
+
+ if (git__add_int_overflow(&old_lineno, hunk->hunk.old_start, hunk->hunk.old_lines) ||
+ git__sub_int_overflow(&old_lineno, old_lineno, oldlines) ||
+ git__add_int_overflow(&new_lineno, hunk->hunk.new_start, hunk->hunk.new_lines) ||
+ git__sub_int_overflow(&new_lineno, new_lineno, newlines)) {
+ error = git_parse_err("unrepresentable line count at line %"PRIuZ,
+ ctx->parse_ctx.line_num);
+ goto done;
+ }
if (ctx->parse_ctx.line_len == 0 || ctx->parse_ctx.line[ctx->parse_ctx.line_len - 1] != '\n') {
error = git_parse_err("invalid patch instruction at line %"PRIuZ,
diff --git a/tests/patch/parse.c b/tests/patch/parse.c
index 77a6dd6..9067f4a 100644
--- a/tests/patch/parse.c
+++ b/tests/patch/parse.c
@@ -174,3 +174,10 @@ void test_patch_parse__truncated_no_newline_at_end_of_file(void)
git_patch_free(patch);
}
+
+void test_patch_parse__line_number_overflow(void)
+{
+ git_patch *patch;
+ cl_git_fail(git_patch_from_buffer(&patch, PATCH_INTMAX_NEW_LINES, strlen(PATCH_INTMAX_NEW_LINES), NULL));
+ git_patch_free(patch);
+}
diff --git a/tests/patch/patch_common.h b/tests/patch/patch_common.h
index 4c053cb..153bab5 100644
--- a/tests/patch/patch_common.h
+++ b/tests/patch/patch_common.h
@@ -918,3 +918,11 @@
"+++ \n" \
"index 0000..7DDb\n" \
"--- \n"
+
+#define PATCH_INTMAX_NEW_LINES \
+ "diff --git a/file b/file\n" \
+ "--- a/file\n" \
+ "+++ b/file\n" \
+ "@@ -0 +2147483647 @@\n" \
+ "\n" \
+ " "