Merge pull request #5276 from pks-t/pks/patch-fuzzing-fixes patch_parse: fixes for fuzzing errors
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 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226
diff --git a/src/integer.h b/src/integer.h
index 4738e9e..e024a86 100644
--- a/src/integer.h
+++ b/src/integer.h
@@ -72,15 +72,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
@@ -108,6 +118,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 1269182..5032e35 100644
--- a/src/patch_parse.c
+++ b/src/patch_parse.c
@@ -69,6 +69,10 @@ static int parse_header_path_buf(git_buf *path, git_patch_parse_ctx *ctx, size_t
{
int error;
+ if (!path_len)
+ return git_parse_err("patch contains empty path at line %"PRIuZ,
+ ctx->parse_ctx.line_num);
+
if ((error = git_buf_put(path, ctx->parse_ctx.line, path_len)) < 0)
goto done;
@@ -91,10 +95,14 @@ done:
static int parse_header_path(char **out, git_patch_parse_ctx *ctx)
{
git_buf path = GIT_BUF_INIT;
- int error = parse_header_path_buf(&path, ctx, header_path_len(ctx));
+ int error;
+ if ((error = parse_header_path_buf(&path, ctx, header_path_len(ctx))) < 0)
+ goto out;
*out = git_buf_detach(&path);
+out:
+ git_buf_dispose(&path);
return error;
}
@@ -104,6 +112,12 @@ static int parse_header_git_oldpath(
git_buf old_path = GIT_BUF_INIT;
int error;
+ if (patch->old_path) {
+ error = git_parse_err("patch contains duplicate old path at line %"PRIuZ,
+ ctx->parse_ctx.line_num);
+ goto out;
+ }
+
if ((error = parse_header_path_buf(&old_path, ctx, ctx->parse_ctx.line_len - 1)) < 0)
goto out;
@@ -120,9 +134,14 @@ static int parse_header_git_newpath(
git_buf new_path = GIT_BUF_INIT;
int error;
- if ((error = parse_header_path_buf(&new_path, ctx, ctx->parse_ctx.line_len - 1)) < 0)
+ if (patch->new_path) {
+ error = git_parse_err("patch contains duplicate new path at line %"PRIuZ,
+ ctx->parse_ctx.line_num);
goto out;
+ }
+ if ((error = parse_header_path_buf(&new_path, ctx, ctx->parse_ctx.line_len - 1)) < 0)
+ goto out;
patch->new_path = git_buf_detach(&new_path);
out:
@@ -564,11 +583,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,
@@ -628,6 +653,7 @@ static int parse_hunk_body(
line->content_len = ctx->parse_ctx.line_len - prefix;
line->content = git__strndup(ctx->parse_ctx.line + prefix, line->content_len);
+ GIT_ERROR_CHECK_ALLOC(line->content);
line->content_offset = ctx->parse_ctx.content_len - ctx->parse_ctx.remain_len;
line->origin = origin;
line->num_lines = 1;
@@ -667,8 +693,9 @@ static int parse_hunk_body(
memset(line, 0x0, sizeof(git_diff_line));
- line->content = git__strdup(ctx->parse_ctx.line);
line->content_len = ctx->parse_ctx.line_len;
+ line->content = git__strndup(ctx->parse_ctx.line, line->content_len);
+ GIT_ERROR_CHECK_ALLOC(line->content);
line->content_offset = ctx->parse_ctx.content_len - ctx->parse_ctx.remain_len;
line->origin = eof_for_origin(last_origin);
line->num_lines = 1;
diff --git a/tests/patch/parse.c b/tests/patch/parse.c
index b89322f..9067f4a 100644
--- a/tests/patch/parse.c
+++ b/tests/patch/parse.c
@@ -148,3 +148,36 @@ void test_patch_parse__lifetime_of_patch_does_not_depend_on_buffer(void)
git_patch_free(patch);
}
+
+void test_patch_parse__binary_file_with_missing_paths(void)
+{
+ git_patch *patch;
+ cl_git_fail(git_patch_from_buffer(&patch, PATCH_BINARY_FILE_WITH_MISSING_PATHS,
+ strlen(PATCH_BINARY_FILE_WITH_MISSING_PATHS), NULL));
+}
+
+void test_patch_parse__memory_leak_on_multiple_paths(void)
+{
+ git_patch *patch;
+ cl_git_fail(git_patch_from_buffer(&patch, PATCH_MULTIPLE_OLD_PATHS, strlen(PATCH_MULTIPLE_OLD_PATHS), NULL));
+}
+
+void test_patch_parse__truncated_no_newline_at_end_of_file(void)
+{
+ size_t len = strlen(PATCH_APPEND_NO_NL) - strlen("at end of file\n");
+ const git_diff_line *line;
+ git_patch *patch;
+
+ cl_git_pass(git_patch_from_buffer(&patch, PATCH_APPEND_NO_NL, len, NULL));
+ cl_git_pass(git_patch_get_line_in_hunk(&line, patch, 0, 4));
+ cl_assert_equal_s(line->content, "\\ No newline ");
+
+ 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 d730d14..153bab5 100644
--- a/tests/patch/patch_common.h
+++ b/tests/patch/patch_common.h
@@ -905,3 +905,24 @@
"-b\n" \
"+bb\n" \
" c\n"
+
+#define PATCH_BINARY_FILE_WITH_MISSING_PATHS \
+ "diff --git \n" \
+ "--- \n" \
+ "+++ \n" \
+ "Binary files "
+
+#define PATCH_MULTIPLE_OLD_PATHS \
+ "diff --git \n" \
+ "--- \n" \
+ "+++ \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" \
+ " "