Be more careful with user-supplied buffers This adds in missing calls to `git_buf_sanitize` and fixes a number of places where `git_buf` APIs could inadvertently write NUL terminator bytes into invalid buffers. This also changes the behavior of `git_buf_sanitize` to NUL terminate a buffer if it can and of `git_buf_shorten` to do nothing if it can. Adds tests of filtering code with zeroed (i.e. unsanitized) buffer which was previously triggering a segfault.
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
diff --git a/src/buffer.c b/src/buffer.c
index 5169c3e..b8f8660 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -104,17 +104,20 @@ void git_buf_free(git_buf *buf)
void git_buf_sanitize(git_buf *buf)
{
if (buf->ptr == NULL) {
- assert (buf->size == 0 && buf->asize == 0);
+ assert(buf->size == 0 && buf->asize == 0);
buf->ptr = git_buf__initbuf;
- }
+ } else if (buf->asize > buf->size)
+ buf->ptr[buf->size] = '\0';
}
void git_buf_clear(git_buf *buf)
{
buf->size = 0;
- if (!buf->ptr)
+ if (!buf->ptr) {
buf->ptr = git_buf__initbuf;
+ buf->asize = 0;
+ }
if (buf->asize > 0)
buf->ptr[0] = '\0';
@@ -129,8 +132,11 @@ int git_buf_set(git_buf *buf, const void *data, size_t len)
ENSURE_SIZE(buf, len + 1);
memmove(buf->ptr, data, len);
}
+
buf->size = len;
- buf->ptr[buf->size] = '\0';
+ if (buf->asize > buf->size)
+ buf->ptr[buf->size] = '\0';
+
}
return 0;
}
@@ -326,19 +332,20 @@ void git_buf_consume(git_buf *buf, const char *end)
void git_buf_truncate(git_buf *buf, size_t len)
{
- if (len < buf->size) {
- buf->size = len;
+ if (len >= buf->size)
+ return;
+
+ buf->size = len;
+ if (buf->size < buf->asize)
buf->ptr[buf->size] = '\0';
- }
}
void git_buf_shorten(git_buf *buf, size_t amount)
{
- if (amount > buf->size)
- amount = buf->size;
-
- buf->size = buf->size - amount;
- buf->ptr[buf->size] = '\0';
+ if (buf->size > amount)
+ git_buf_truncate(buf, buf->size - amount);
+ else
+ git_buf_clear(buf);
}
void git_buf_rtruncate_at_char(git_buf *buf, char separator)
@@ -574,7 +581,8 @@ void git_buf_rtrim(git_buf *buf)
buf->size--;
}
- buf->ptr[buf->size] = '\0';
+ if (buf->asize > buf->size)
+ buf->ptr[buf->size] = '\0';
}
int git_buf_cmp(const git_buf *a, const git_buf *b)
@@ -598,8 +606,7 @@ int git_buf_splice(
/* Ported from git.git
* https://github.com/git/git/blob/16eed7c/strbuf.c#L159-176
*/
- if (git_buf_grow(buf, git_buf_len(buf) + nb_to_insert - nb_to_remove) < 0)
- return -1;
+ ENSURE_SIZE(buf, buf->size + nb_to_insert - nb_to_insert + 1);
memmove(buf->ptr + where + nb_to_insert,
buf->ptr + where + nb_to_remove,
diff --git a/src/config.c b/src/config.c
index f9d6971..4dddab6 100644
--- a/src/config.c
+++ b/src/config.c
@@ -967,16 +967,19 @@ void git_config_iterator_free(git_config_iterator *iter)
int git_config_find_global(git_buf *path)
{
+ git_buf_sanitize(path);
return git_sysdir_find_global_file(path, GIT_CONFIG_FILENAME_GLOBAL);
}
int git_config_find_xdg(git_buf *path)
{
+ git_buf_sanitize(path);
return git_sysdir_find_xdg_file(path, GIT_CONFIG_FILENAME_XDG);
}
int git_config_find_system(git_buf *path)
{
+ git_buf_sanitize(path);
return git_sysdir_find_system_file(path, GIT_CONFIG_FILENAME_SYSTEM);
}
diff --git a/src/diff_print.c b/src/diff_print.c
index 07c1f85..08e1e7f 100644
--- a/src/diff_print.c
+++ b/src/diff_print.c
@@ -597,9 +597,9 @@ int git_diff_print_callback__to_file_handle(
}
/* print a git_patch to a git_buf */
-int git_patch_to_buf(
- git_buf *out,
- git_patch *patch)
+int git_patch_to_buf(git_buf *out, git_patch *patch)
{
+ assert(out && patch);
+ git_buf_sanitize(out);
return git_patch_print(patch, git_diff_print_callback__to_buf, out);
}
diff --git a/src/filter.c b/src/filter.c
index b2f5796..3c36aaf 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -578,6 +578,9 @@ int git_filter_list_apply_to_data(
git_buf *dbuffer[2], local = GIT_BUF_INIT;
unsigned int si = 0;
+ git_buf_sanitize(tgt);
+ git_buf_sanitize(src);
+
if (!fl)
return filter_list_out_buffer_from_raw(tgt, src->ptr, src->size);
@@ -613,7 +616,7 @@ int git_filter_list_apply_to_data(
/* PASSTHROUGH means filter decided not to process the buffer */
error = 0;
} else if (!error) {
- git_buf_shorten(dbuffer[di], 0); /* force NUL termination */
+ git_buf_sanitize(dbuffer[di]); /* force NUL termination */
si = di; /* swap buffers */
} else {
tgt->size = 0;
diff --git a/src/pack-objects.c b/src/pack-objects.c
index ace8afd..b503385 100644
--- a/src/pack-objects.c
+++ b/src/pack-objects.c
@@ -1276,6 +1276,7 @@ int git_packbuilder_foreach(git_packbuilder *pb, int (*cb)(void *buf, size_t siz
int git_packbuilder_write_buf(git_buf *buf, git_packbuilder *pb)
{
PREPARE_PACK;
+ git_buf_sanitize(buf);
return write_pack(pb, &write_pack_buf, buf);
}
diff --git a/src/submodule.c b/src/submodule.c
index 5ddbfe8..b1291df 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -641,7 +641,9 @@ int git_submodule_resolve_url(git_buf *out, git_repository *repo, const char *ur
{
int error = 0;
- assert(url);
+ assert(out && repo && url);
+
+ git_buf_sanitize(out);
if (git_path_is_relative(url)) {
if (!(error = get_url_base(out, repo)))
diff --git a/tests/object/blob/filter.c b/tests/object/blob/filter.c
index 0b2d6bf..3cc8fdc 100644
--- a/tests/object/blob/filter.c
+++ b/tests/object/blob/filter.c
@@ -112,7 +112,7 @@ void test_object_blob_filter__to_odb(void)
git_config *cfg;
int i;
git_blob *blob;
- git_buf out = GIT_BUF_INIT;
+ git_buf out = GIT_BUF_INIT, zeroed;
cl_git_pass(git_repository_config(&cfg, g_repo));
cl_assert(cfg);
@@ -127,13 +127,20 @@ void test_object_blob_filter__to_odb(void)
for (i = 0; i < CRLF_NUM_TEST_OBJECTS; i++) {
cl_git_pass(git_blob_lookup(&blob, g_repo, &g_crlf_oids[i]));
+ /* try once with allocated blob */
cl_git_pass(git_filter_list_apply_to_blob(&out, fl, blob));
-
cl_assert_equal_sz(g_crlf_filtered[i].size, out.size);
-
cl_assert_equal_i(
0, memcmp(out.ptr, g_crlf_filtered[i].ptr, out.size));
+ /* try again with zeroed blob */
+ memset(&zeroed, 0, sizeof(zeroed));
+ cl_git_pass(git_filter_list_apply_to_blob(&zeroed, fl, blob));
+ cl_assert_equal_sz(g_crlf_filtered[i].size, zeroed.size);
+ cl_assert_equal_i(
+ 0, memcmp(zeroed.ptr, g_crlf_filtered[i].ptr, zeroed.size));
+ git_buf_free(&zeroed);
+
git_blob_free(blob);
}