thodg/libgit2/tests/patch/parse.c
Tag
-
Show log
Commit
-
Hash : 33e6c402
Author :
Date : 2019-11-28T15:26:36
patch_parse: fix out-of-bounds reads caused by integer underflow The patch format for binary files is a simple Base85 encoding with a length byte as prefix that encodes the current line's length. For each line, we thus check whether the line's actual length matches its expected length in order to not faultily apply a truncated patch. This also acts as a check to verify that we're not reading outside of the line's string: if (encoded_len > ctx->parse_ctx.line_len - 1) { error = git_parse_err(...); goto done; } There is the possibility for an integer underflow, though. Given a line with a single prefix byte, only, `line_len` will be zero when reaching this check. As a result, subtracting one from that will result in an integer underflow, causing us to assume that there's a wealth of bytes available later on. Naturally, this may result in an out-of-bounds read. Fix the issue by checking both `encoded_len` and `line_len` for a non-zero value. The binary format doesn't make use of zero-length lines anyway, so we need to know that there are both encoded bytes and remaining characters available at all. This patch also adds a test that works based on the last error message. Checking error messages is usually too tightly coupled, but in fact parsing the patch failed even before the change. Thus the only possibility is to use e.g. Valgrind, but that'd result in us not catching issues when run without Valgrind. As a result, using the error message is considered a viable tradeoff as we know that we didn't start decoding Base85 in the first place.
-
tests/patch/parse.c
-
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
#include "clar_libgit2.h" #include "patch.h" #include "patch_parse.h" #include "patch_common.h" static void ensure_patch_validity(git_patch *patch) { const git_diff_delta *delta; char idstr[GIT_OID_HEXSZ+1] = {0}; cl_assert((delta = git_patch_get_delta(patch)) != NULL); cl_assert_equal_i(2, delta->nfiles); cl_assert_equal_s(delta->old_file.path, "file.txt"); cl_assert(delta->old_file.mode == GIT_FILEMODE_BLOB); cl_assert_equal_i(7, delta->old_file.id_abbrev); git_oid_nfmt(idstr, delta->old_file.id_abbrev, &delta->old_file.id); cl_assert_equal_s(idstr, "9432026"); cl_assert_equal_i(0, delta->old_file.size); cl_assert_equal_s(delta->new_file.path, "file.txt"); cl_assert(delta->new_file.mode == GIT_FILEMODE_BLOB); cl_assert_equal_i(7, delta->new_file.id_abbrev); git_oid_nfmt(idstr, delta->new_file.id_abbrev, &delta->new_file.id); cl_assert_equal_s(idstr, "cd8fd12"); cl_assert_equal_i(0, delta->new_file.size); } static void ensure_identical_patch_inout(const char *content) { git_buf buf = GIT_BUF_INIT; git_patch *patch; cl_git_pass(git_patch_from_buffer(&patch, content, strlen(content), NULL)); cl_git_pass(git_patch_to_buf(&buf, patch)); cl_assert_equal_strn(git_buf_cstr(&buf), content, strlen(content)); git_patch_free(patch); git_buf_dispose(&buf); } void test_patch_parse__original_to_change_middle(void) { git_patch *patch; cl_git_pass(git_patch_from_buffer( &patch, PATCH_ORIGINAL_TO_CHANGE_MIDDLE, strlen(PATCH_ORIGINAL_TO_CHANGE_MIDDLE), NULL)); ensure_patch_validity(patch); git_patch_free(patch); } void test_patch_parse__leading_and_trailing_garbage(void) { git_patch *patch; const char *leading = "This is some leading garbage.\n" "Maybe it's email headers?\n" "\n" PATCH_ORIGINAL_TO_CHANGE_MIDDLE; const char *trailing = PATCH_ORIGINAL_TO_CHANGE_MIDDLE "\n" "This is some trailing garbage.\n" "Maybe it's an email signature?\n"; const char *both = "Here's some leading garbage\n" PATCH_ORIGINAL_TO_CHANGE_MIDDLE "And here's some trailing.\n"; cl_git_pass(git_patch_from_buffer(&patch, leading, strlen(leading), NULL)); ensure_patch_validity(patch); git_patch_free(patch); cl_git_pass(git_patch_from_buffer(&patch, trailing, strlen(trailing), NULL)); ensure_patch_validity(patch); git_patch_free(patch); cl_git_pass(git_patch_from_buffer(&patch, both, strlen(both), NULL)); ensure_patch_validity(patch); git_patch_free(patch); } void test_patch_parse__nonpatches_fail_with_notfound(void) { git_patch *patch; cl_git_fail_with(GIT_ENOTFOUND, git_patch_from_buffer(&patch, PATCH_NOT_A_PATCH, strlen(PATCH_NOT_A_PATCH), NULL)); } void test_patch_parse__invalid_patches_fails(void) { git_patch *patch; cl_git_fail_with(GIT_ERROR, git_patch_from_buffer(&patch, PATCH_CORRUPT_GIT_HEADER, strlen(PATCH_CORRUPT_GIT_HEADER), NULL)); cl_git_fail_with(GIT_ERROR, git_patch_from_buffer(&patch, PATCH_CORRUPT_MISSING_NEW_FILE, strlen(PATCH_CORRUPT_MISSING_NEW_FILE), NULL)); cl_git_fail_with(GIT_ERROR, git_patch_from_buffer(&patch, PATCH_CORRUPT_MISSING_OLD_FILE, strlen(PATCH_CORRUPT_MISSING_OLD_FILE), NULL)); cl_git_fail_with(GIT_ERROR, git_patch_from_buffer(&patch, PATCH_CORRUPT_NO_CHANGES, strlen(PATCH_CORRUPT_NO_CHANGES), NULL)); cl_git_fail_with(GIT_ERROR, git_patch_from_buffer(&patch, PATCH_CORRUPT_MISSING_HUNK_HEADER, strlen(PATCH_CORRUPT_MISSING_HUNK_HEADER), NULL)); } void test_patch_parse__no_newline_at_end_of_new_file(void) { ensure_identical_patch_inout(PATCH_APPEND_NO_NL); } void test_patch_parse__no_newline_at_end_of_old_file(void) { ensure_identical_patch_inout(PATCH_APPEND_NO_NL_IN_OLD_FILE); } void test_patch_parse__files_with_whitespaces_succeeds(void) { ensure_identical_patch_inout(PATCH_NAME_WHITESPACE); } void test_patch_parse__lifetime_of_patch_does_not_depend_on_buffer(void) { git_buf diff = GIT_BUF_INIT, rendered = GIT_BUF_INIT; git_patch *patch; cl_git_pass(git_buf_sets(&diff, PATCH_ORIGINAL_TO_CHANGE_MIDDLE)); cl_git_pass(git_patch_from_buffer(&patch, diff.ptr, diff.size, NULL)); git_buf_dispose(&diff); cl_git_pass(git_patch_to_buf(&rendered, patch)); cl_assert_equal_s(PATCH_ORIGINAL_TO_CHANGE_MIDDLE, rendered.ptr); git_buf_dispose(&rendered); cl_git_pass(git_patch_to_buf(&rendered, patch)); cl_assert_equal_s(PATCH_ORIGINAL_TO_CHANGE_MIDDLE, rendered.ptr); git_buf_dispose(&rendered); 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__binary_file_with_whitespace_paths(void) { git_patch *patch; cl_git_fail(git_patch_from_buffer(&patch, PATCH_BINARY_FILE_WITH_WHITESPACE_PATHS, strlen(PATCH_BINARY_FILE_WITH_WHITESPACE_PATHS), NULL)); } void test_patch_parse__binary_file_with_empty_quoted_paths(void) { git_patch *patch; cl_git_fail(git_patch_from_buffer(&patch, PATCH_BINARY_FILE_WITH_QUOTED_EMPTY_PATHS, strlen(PATCH_BINARY_FILE_WITH_QUOTED_EMPTY_PATHS), NULL)); } void test_patch_parse__binary_file_path_with_spaces(void) { git_patch *patch; cl_git_fail(git_patch_from_buffer(&patch, PATCH_BINARY_FILE_PATH_WITH_SPACES, strlen(PATCH_BINARY_FILE_PATH_WITH_SPACES), NULL)); } void test_patch_parse__binary_file_path_without_body_paths(void) { git_patch *patch; cl_git_fail(git_patch_from_buffer(&patch, PATCH_BINARY_FILE_PATH_WITHOUT_BODY_PATHS, strlen(PATCH_BINARY_FILE_PATH_WITHOUT_BODY_PATHS), NULL)); } void test_patch_parse__binary_file_with_truncated_delta(void) { git_patch *patch; cl_git_fail(git_patch_from_buffer(&patch, PATCH_BINARY_FILE_WITH_TRUNCATED_DELTA, strlen(PATCH_BINARY_FILE_WITH_TRUNCATED_DELTA), NULL)); cl_assert_equal_s(git_error_last()->message, "truncated binary data at line 5"); } 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); }