Skip to content

Commit 392702e

Browse files
Edward Thomsonethomson
Edward Thomson
authored and
ethomson
committed
allocations: test for overflow of requested size
Introduce some helper macros to test integer overflow from arithmetic and set error message appropriately.
1 parent d24a531 commit 392702e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+528
-117
lines changed

‎src/array.h‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,28 @@ typedef git_array_t(char) git_array_generic_t;
4545
GIT_INLINE(void *) git_array_grow(void *_a, size_t item_size)
4646
{
4747
volatile git_array_generic_t *a = _a;
48-
uint32_t new_size = (a->size < 8) ? 8 : a->asize * 3 / 2;
49-
char *new_array = git__realloc(a->ptr, new_size * item_size);
50-
if (!new_array) {
51-
git_array_clear(*a);
52-
return NULL;
48+
uint32_t new_size;
49+
char *new_array;
50+
51+
if (a->size < 8) {
52+
new_size = 8;
5353
} else {
54-
a->ptr = new_array; a->asize = new_size; a->size++;
55-
return a->ptr + (a->size - 1) * item_size;
54+
if (GIT_ALLOC_OVERFLOW_MULTIPLY(a->size, 3 / 2))
55+
goto on_oom;
56+
57+
new_size = a->size * 3 / 2;
5658
}
59+
60+
if (GIT_ALLOC_OVERFLOW_MULTIPLY(new_size, item_size) ||
61+
(new_array = git__realloc(a->ptr, new_size * item_size)) == NULL)
62+
goto on_oom;
63+
64+
a->ptr = new_array; a->asize = new_size; a->size++;
65+
return a->ptr + (a->size - 1) * item_size;
66+
67+
on_oom:
68+
git_array_clear(*a);
69+
return NULL;
5770
}
5871

5972
#define git_array_alloc(a) \

‎src/blame.c‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ static git_blame_hunk* dup_hunk(git_blame_hunk *hunk)
7676
hunk->lines_in_hunk,
7777
hunk->orig_start_line_number,
7878
hunk->orig_path);
79+
80+
if (!newhunk)
81+
return NULL;
82+
7983
git_oid_cpy(&newhunk->orig_commit_id, &hunk->orig_commit_id);
8084
git_oid_cpy(&newhunk->final_commit_id, &hunk->final_commit_id);
8185
newhunk->boundary = hunk->boundary;
@@ -221,6 +225,10 @@ static git_blame_hunk *split_hunk_in_vector(
221225
new_line_count = hunk->lines_in_hunk - rel_line;
222226
nh = new_hunk((uint16_t)(hunk->final_start_line_number+rel_line), (uint16_t)new_line_count,
223227
(uint16_t)(hunk->orig_start_line_number+rel_line), hunk->orig_path);
228+
229+
if (!nh)
230+
return NULL;
231+
224232
git_oid_cpy(&nh->final_commit_id, &hunk->final_commit_id);
225233
git_oid_cpy(&nh->orig_commit_id, &hunk->orig_commit_id);
226234

@@ -270,6 +278,10 @@ static git_blame_hunk* hunk_from_entry(git_blame__entry *e)
270278
{
271279
git_blame_hunk *h = new_hunk(
272280
e->lno+1, e->num_lines, e->s_lno+1, e->suspect->path);
281+
282+
if (!h)
283+
return NULL;
284+
273285
git_oid_cpy(&h->final_commit_id, git_commit_id(e->suspect->commit));
274286
git_oid_cpy(&h->orig_commit_id, git_commit_id(e->suspect->commit));
275287
git_signature_dup(&h->final_signature, git_commit_author(e->suspect->commit));
@@ -307,6 +319,8 @@ static int blame_internal(git_blame *blame)
307319
blame->final_buf_size = git_blob_rawsize(blame->final_blob);
308320

309321
ent = git__calloc(1, sizeof(git_blame__entry));
322+
GITERR_CHECK_ALLOC(ent);
323+
310324
ent->num_lines = index_blob_lines(blame);
311325
ent->lno = blame->options.min_line - 1;
312326
ent->num_lines = ent->num_lines - blame->options.min_line + 1;
@@ -322,8 +336,9 @@ static int blame_internal(git_blame *blame)
322336
cleanup:
323337
for (ent = blame->ent; ent; ) {
324338
git_blame__entry *e = ent->next;
339+
git_blame_hunk *h = hunk_from_entry(ent);
325340

326-
git_vector_insert(&blame->hunks, hunk_from_entry(ent));
341+
git_vector_insert(&blame->hunks, h);
327342

328343
git_blame__free_entry(ent);
329344
ent = e;
@@ -392,11 +407,14 @@ static int buffer_hunk_cb(
392407
if (!blame->current_hunk) {
393408
/* Line added at the end of the file */
394409
blame->current_hunk = new_hunk(wedge_line, 0, wedge_line, blame->path);
410+
GITERR_CHECK_ALLOC(blame->current_hunk);
411+
395412
git_vector_insert(&blame->hunks, blame->current_hunk);
396413
} else if (!hunk_starts_at_or_after_line(blame->current_hunk, wedge_line)){
397414
/* If this hunk doesn't start between existing hunks, split a hunk up so it does */
398415
blame->current_hunk = split_hunk_in_vector(&blame->hunks, blame->current_hunk,
399416
wedge_line - blame->current_hunk->orig_start_line_number, true);
417+
GITERR_CHECK_ALLOC(blame->current_hunk);
400418
}
401419

402420
return 0;
@@ -425,6 +443,8 @@ static int buffer_line_cb(
425443
/* Create a new buffer-blame hunk with this line */
426444
shift_hunks_by(&blame->hunks, blame->current_diff_line, 1);
427445
blame->current_hunk = new_hunk((uint16_t)blame->current_diff_line, 1, 0, blame->path);
446+
GITERR_CHECK_ALLOC(blame->current_hunk);
447+
428448
git_vector_insert_sorted(&blame->hunks, blame->current_hunk, NULL);
429449
}
430450
blame->current_diff_line++;
@@ -464,10 +484,14 @@ int git_blame_buffer(
464484
assert(out && reference && buffer && buffer_len);
465485

466486
blame = git_blame__alloc(reference->repository, reference->options, reference->path);
487+
GITERR_CHECK_ALLOC(blame);
467488

468489
/* Duplicate all of the hunk structures in the reference blame */
469490
git_vector_foreach(&reference->hunks, i, hunk) {
470-
git_vector_insert(&blame->hunks, dup_hunk(hunk));
491+
git_blame_hunk *h = dup_hunk(hunk);
492+
GITERR_CHECK_ALLOC(h);
493+
494+
git_vector_insert(&blame->hunks, h);
471495
}
472496

473497
/* Diff to the reference blob */

‎src/blame_git.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,14 @@ static void origin_decref(git_blame__origin *o)
3535
/* Given a commit and a path in it, create a new origin structure. */
3636
static int make_origin(git_blame__origin **out, git_commit *commit, const char *path)
3737
{
38-
int error = 0;
3938
git_blame__origin *o;
39+
size_t path_len = strlen(path);
40+
int error = 0;
41+
42+
GITERR_CHECK_ALLOC_ADD(sizeof(*o), path_len);
43+
GITERR_CHECK_ALLOC_ADD(sizeof(*o) + path_len, 1);
4044

41-
o = git__calloc(1, sizeof(*o) + strlen(path) + 1);
45+
o = git__calloc(1, sizeof(*o) + path_len + 1);
4246
GITERR_CHECK_ALLOC(o);
4347
o->commit = commit;
4448
o->refcnt = 1;

‎src/buf_text.c‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ int git_buf_text_puts_escaped(
2929
scan += count;
3030
}
3131

32+
GITERR_CHECK_ALLOC_ADD(buf->size, total);
33+
GITERR_CHECK_ALLOC_ADD(buf->size + total, 1);
3234
if (git_buf_grow(buf, buf->size + total + 1) < 0)
3335
return -1;
3436

@@ -73,8 +75,10 @@ int git_buf_text_crlf_to_lf(git_buf *tgt, const git_buf *src)
7375
return git_buf_set(tgt, src->ptr, src->size);
7476

7577
/* reduce reallocs while in the loop */
78+
GITERR_CHECK_ALLOC_ADD(src->size, 1);
7679
if (git_buf_grow(tgt, src->size + 1) < 0)
7780
return -1;
81+
7882
out = tgt->ptr;
7983
tgt->size = 0;
8084

@@ -117,20 +121,26 @@ int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src)
117121
return git_buf_set(tgt, src->ptr, src->size);
118122

119123
/* attempt to reduce reallocs while in the loop */
124+
GITERR_CHECK_ALLOC_ADD(src->size, src->size >> 4);
125+
GITERR_CHECK_ALLOC_ADD(src->size + (src->size >> 4), 1);
120126
if (git_buf_grow(tgt, src->size + (src->size >> 4) + 1) < 0)
121127
return -1;
122128
tgt->size = 0;
123129

124130
for (; next; scan = next + 1, next = memchr(scan, '\n', end - scan)) {
125131
size_t copylen = next - scan;
126-
size_t needsize = tgt->size + copylen + 2 + 1;
132+
size_t needsize;
127133

128134
/* if we find mixed line endings, bail */
129135
if (next > start && next[-1] == '\r') {
130136
git_buf_free(tgt);
131137
return GIT_PASSTHROUGH;
132138
}
133139

140+
GITERR_CHECK_ALLOC_ADD(tgt->size, copylen);
141+
GITERR_CHECK_ALLOC_ADD(tgt->size + copylen, 3);
142+
needsize = tgt->size + copylen + 3;
143+
134144
if (tgt->asize < needsize && git_buf_grow(tgt, needsize) < 0)
135145
return -1;
136146

‎src/buffer.c‎

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,14 @@ int git_buf_try_grow(
6363
/* round allocation up to multiple of 8 */
6464
new_size = (new_size + 7) & ~7;
6565

66+
if (new_size < buf->size) {
67+
if (mark_oom)
68+
buf->ptr = git_buf__oom;
69+
70+
giterr_set_oom();
71+
return -1;
72+
}
73+
6674
new_ptr = git__realloc(new_ptr, new_size);
6775

6876
if (!new_ptr) {
@@ -131,6 +139,7 @@ int git_buf_set(git_buf *buf, const void *data, size_t len)
131139
git_buf_clear(buf);
132140
} else {
133141
if (data != buf->ptr) {
142+
GITERR_CHECK_ALLOC_ADD(len, 1);
134143
ENSURE_SIZE(buf, len + 1);
135144
memmove(buf->ptr, data, len);
136145
}
@@ -160,6 +169,7 @@ int git_buf_sets(git_buf *buf, const char *string)
160169

161170
int git_buf_putc(git_buf *buf, char c)
162171
{
172+
GITERR_CHECK_ALLOC_ADD(buf->size, 2);
163173
ENSURE_SIZE(buf, buf->size + 2);
164174
buf->ptr[buf->size++] = c;
165175
buf->ptr[buf->size] = '\0';
@@ -168,6 +178,8 @@ int git_buf_putc(git_buf *buf, char c)
168178

169179
int git_buf_putcn(git_buf *buf, char c, size_t len)
170180
{
181+
GITERR_CHECK_ALLOC_ADD(buf->size, len);
182+
GITERR_CHECK_ALLOC_ADD(buf->size + len, 1);
171183
ENSURE_SIZE(buf, buf->size + len + 1);
172184
memset(buf->ptr + buf->size, c, len);
173185
buf->size += len;
@@ -179,6 +191,8 @@ int git_buf_put(git_buf *buf, const char *data, size_t len)
179191
{
180192
if (len) {
181193
assert(data);
194+
GITERR_CHECK_ALLOC_ADD(buf->size, len);
195+
GITERR_CHECK_ALLOC_ADD(buf->size + len, 1);
182196
ENSURE_SIZE(buf, buf->size + len + 1);
183197
memmove(buf->ptr + buf->size, data, len);
184198
buf->size += len;
@@ -201,8 +215,12 @@ int git_buf_encode_base64(git_buf *buf, const char *data, size_t len)
201215
size_t extra = len % 3;
202216
uint8_t *write, a, b, c;
203217
const uint8_t *read = (const uint8_t *)data;
218+
size_t blocks = (len / 3) + !!extra;
204219

205-
ENSURE_SIZE(buf, buf->size + 4 * ((len / 3) + !!extra) + 1);
220+
GITERR_CHECK_ALLOC_MULTIPLY(blocks, 4);
221+
GITERR_CHECK_ALLOC_ADD(buf->size, 4 * blocks);
222+
GITERR_CHECK_ALLOC_ADD(buf->size + 4 * blocks, 1);
223+
ENSURE_SIZE(buf, buf->size + 4 * blocks + 1);
206224
write = (uint8_t *)&buf->ptr[buf->size];
207225

208226
/* convert each run of 3 bytes into 4 output bytes */
@@ -256,6 +274,8 @@ int git_buf_decode_base64(git_buf *buf, const char *base64, size_t len)
256274
size_t orig_size = buf->size;
257275

258276
assert(len % 4 == 0);
277+
GITERR_CHECK_ALLOC_ADD(buf->size, len / 4 * 3);
278+
GITERR_CHECK_ALLOC_ADD(buf->size + (len / 4 * 3), 1);
259279
ENSURE_SIZE(buf, buf->size + (len / 4 * 3) + 1);
260280

261281
for (i = 0; i < len; i += 4) {
@@ -284,7 +304,12 @@ static const char b85str[] =
284304

285305
int git_buf_encode_base85(git_buf *buf, const char *data, size_t len)
286306
{
287-
ENSURE_SIZE(buf, buf->size + (5 * ((len / 4) + !!(len % 4))) + 1);
307+
size_t blocks = (len / 4) + !!(len % 4);
308+
309+
GITERR_CHECK_ALLOC_MULTIPLY(blocks, 5);
310+
GITERR_CHECK_ALLOC_ADD(buf->size, 5 * blocks);
311+
GITERR_CHECK_ALLOC_ADD(buf->size + 5 * blocks, 1);
312+
ENSURE_SIZE(buf, buf->size + blocks * 5 + 1);
288313

289314
while (len) {
290315
uint32_t acc = 0;
@@ -317,8 +342,14 @@ int git_buf_encode_base85(git_buf *buf, const char *data, size_t len)
317342

318343
int git_buf_vprintf(git_buf *buf, const char *format, va_list ap)
319344
{
345+
size_t expected_size = strlen(format);
320346
int len;
321-
const size_t expected_size = buf->size + (strlen(format) * 2);
347+
348+
GITERR_CHECK_ALLOC_MULTIPLY(expected_size, 2);
349+
expected_size *= 2;
350+
351+
GITERR_CHECK_ALLOC_ADD(expected_size, buf->size);
352+
expected_size += buf->size;
322353

323354
ENSURE_SIZE(buf, expected_size);
324355

@@ -345,6 +376,8 @@ int git_buf_vprintf(git_buf *buf, const char *format, va_list ap)
345376
break;
346377
}
347378

379+
GITERR_CHECK_ALLOC_ADD(buf->size, len);
380+
GITERR_CHECK_ALLOC_ADD(buf->size + len, 1);
348381
ENSURE_SIZE(buf, buf->size + len + 1);
349382
}
350383

@@ -481,6 +514,9 @@ int git_buf_join_n(git_buf *buf, char separator, int nbuf, ...)
481514
/* expand buffer if needed */
482515
if (total_size == 0)
483516
return 0;
517+
518+
GITERR_CHECK_ALLOC_ADD(buf->size, total_size);
519+
GITERR_CHECK_ALLOC_ADD(buf->size + total_size, 1);
484520
if (git_buf_grow(buf, buf->size + total_size + 1) < 0)
485521
return -1;
486522

@@ -559,6 +595,9 @@ int git_buf_join(
559595
if (str_a >= buf->ptr && str_a < buf->ptr + buf->size)
560596
offset_a = str_a - buf->ptr;
561597

598+
GITERR_CHECK_ALLOC_ADD(strlen_a, strlen_b);
599+
GITERR_CHECK_ALLOC_ADD(strlen_a + strlen_b, need_sep);
600+
GITERR_CHECK_ALLOC_ADD(strlen_a + strlen_b + need_sep, 1);
562601
if (git_buf_grow(buf, strlen_a + strlen_b + need_sep + 1) < 0)
563602
return -1;
564603
assert(buf->ptr);
@@ -607,6 +646,11 @@ int git_buf_join3(
607646
sep_b = (str_b[len_b - 1] != separator);
608647
}
609648

649+
GITERR_CHECK_ALLOC_ADD(len_a, sep_a);
650+
GITERR_CHECK_ALLOC_ADD(len_a + sep_a, len_b);
651+
GITERR_CHECK_ALLOC_ADD(len_a + sep_a + len_b, sep_b);
652+
GITERR_CHECK_ALLOC_ADD(len_a + sep_a + len_b + sep_b, len_c);
653+
GITERR_CHECK_ALLOC_ADD(len_a + sep_a + len_b + sep_b + len_c, 1);
610654
if (git_buf_grow(buf, len_a + sep_a + len_b + sep_b + len_c + 1) < 0)
611655
return -1;
612656

@@ -660,22 +704,30 @@ int git_buf_splice(
660704
const char *data,
661705
size_t nb_to_insert)
662706
{
707+
size_t new_size;
708+
663709
assert(buf &&
664710
where <= git_buf_len(buf) &&
665711
where + nb_to_remove <= git_buf_len(buf));
666712

667713
/* Ported from git.git
668714
* https://github.com/git/git/blob/16eed7c/strbuf.c#L159-176
669715
*/
670-
ENSURE_SIZE(buf, buf->size + nb_to_insert - nb_to_insert + 1);
716+
new_size = buf->size - nb_to_remove;
717+
718+
GITERR_CHECK_ALLOC_ADD(new_size, nb_to_insert);
719+
new_size += nb_to_insert;
720+
721+
GITERR_CHECK_ALLOC_ADD(new_size, 1);
722+
ENSURE_SIZE(buf, new_size + 1);
671723

672724
memmove(buf->ptr + where + nb_to_insert,
673725
buf->ptr + where + nb_to_remove,
674726
buf->size - where - nb_to_remove);
675727

676728
memcpy(buf->ptr + where, data, nb_to_insert);
677729

678-
buf->size = buf->size + nb_to_insert - nb_to_remove;
730+
buf->size = new_size;
679731
buf->ptr[buf->size] = '\0';
680732
return 0;
681733
}

‎src/common.h‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,28 @@ GIT_INLINE(void) git__init_structure(void *structure, size_t len, unsigned int v
174174
GITERR_CHECK_VERSION(&(VERSION), _tmpl.version, #TYPE); \
175175
memcpy((PTR), &_tmpl, sizeof(_tmpl)); } while (0)
176176

177+
/** Check for integer overflow from addition or multiplication */
178+
#define GIT_ALLOC_OVERFLOW_ADD(one, two) \
179+
((one) + (two) < (one))
180+
181+
/** Check for integer overflow from multiplication */
182+
#define GIT_ALLOC_OVERFLOW_MULTIPLY(one, two) \
183+
(one && ((one) * (two)) / (one) != (two))
184+
185+
/** Check for additive overflow, failing if it would occur. */
186+
#define GITERR_CHECK_ALLOC_ADD(one, two) \
187+
if (GIT_ALLOC_OVERFLOW_ADD(one, two)) { \
188+
giterr_set_oom(); \
189+
return -1; \
190+
}
191+
192+
/** Check for multiplicative overflow, failing if it would occur. */
193+
#define GITERR_CHECK_ALLOC_MULTIPLY(nelem, elsize) \
194+
if (GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize)) { \
195+
giterr_set_oom(); \
196+
return -1; \
197+
}
198+
177199
/* NOTE: other giterr functions are in the public errors.h header file */
178200

179201
#include "util.h"

0 commit comments

Comments
 (0)