diff options
author | Patrick Steinhardt <ps@pks.im> | 2019-10-29 07:52:31 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-10-29 07:52:31 +0100 |
commit | 2a7d6de3350c9479b1094cf1ce96ee4c9c3b6c3c (patch) | |
tree | e9675e060de78b0bb7a4da6389f41bed3e15281d | |
parent | a31f4c4b539a6c4bc7cf9b8bd71c27f9f3935919 (diff) | |
parent | 37141ff7701e45ab0d97f311a4e0cc95cf527aa9 (diff) | |
download | libgit2-2a7d6de3350c9479b1094cf1ce96ee4c9c3b6c3c.tar.gz |
Merge pull request #5276 from pks-t/pks/patch-fuzzing-fixes
patch_parse: fixes for fuzzing errors
-rw-r--r-- | src/integer.h | 28 | ||||
-rw-r--r-- | src/patch_parse.c | 41 | ||||
-rw-r--r-- | tests/patch/parse.c | 33 | ||||
-rw-r--r-- | tests/patch/patch_common.h | 21 |
4 files changed, 116 insertions, 7 deletions
diff --git a/src/integer.h b/src/integer.h index 4738e9e35..e024a86d3 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 126918249..5032e35c8 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 b89322ff3..9067f4a9d 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 d730d142c..153bab57f 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" \ + " " |