Skip to content

Commit 568a05c

Browse files
rscharfegitster
authored andcommitted
cleanup: fix possible overflow errors in binary search, part 2
Calculating the sum of two array indexes to find the midpoint between them can overflow, i.e. code like this is unsafe for big arrays: mid = (first + last) >> 1; Make sure the intermediate value stays within the boundaries instead, like this: mid = first + ((last - first) >> 1); The loop condition of the binary search makes sure that 'last' is always greater than 'first', so this is safe as long as 'first' is not negative. And that can be verified easily using the pre-context of each change, except for name-hash.c, so add an assertion to that effect there. The unsafe calculations were found with: git grep '(.*+.*) *>> *1' This is a continuation of 19716b2 (cleanup: fix possible overflow errors in binary search, 2017-10-08). Signed-off-by: Rene Scharfe <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent b697d92 commit 568a05c

6 files changed

+8
-7
lines changed

builtin/ls-files.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ static void prune_index(struct index_state *istate,
373373
first = pos;
374374
last = istate->cache_nr;
375375
while (last > first) {
376-
int next = (last + first) >> 1;
376+
int next = first + ((last - first) >> 1);
377377
const struct cache_entry *ce = istate->cache[next];
378378
if (!strncmp(ce->name, prefix, prefixlen)) {
379379
first = next+1;

diffcore-rename.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ static int find_rename_dst(struct diff_filespec *two)
2323
first = 0;
2424
last = rename_dst_nr;
2525
while (last > first) {
26-
int next = (last + first) >> 1;
26+
int next = first + ((last - first) >> 1);
2727
struct diff_rename_dst *dst = &(rename_dst[next]);
2828
int cmp = strcmp(two->path, dst->two->path);
2929
if (!cmp)
@@ -83,7 +83,7 @@ static struct diff_rename_src *register_rename_src(struct diff_filepair *p)
8383
first = 0;
8484
last = rename_src_nr;
8585
while (last > first) {
86-
int next = (last + first) >> 1;
86+
int next = first + ((last - first) >> 1);
8787
struct diff_rename_src *src = &(rename_src[next]);
8888
int cmp = strcmp(one->path, src->p->one->path);
8989
if (!cmp)

dir.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -701,7 +701,7 @@ static struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,
701701
first = 0;
702702
last = dir->dirs_nr;
703703
while (last > first) {
704-
int cmp, next = (last + first) >> 1;
704+
int cmp, next = first + ((last - first) >> 1);
705705
d = dir->dirs[next];
706706
cmp = strncmp(name, d->name, len);
707707
if (!cmp && strlen(d->name) > len)

name-hash.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -345,8 +345,9 @@ static int handle_range_dir(
345345
else {
346346
int begin = k_start;
347347
int end = k_end;
348+
assert(begin >= 0);
348349
while (begin < end) {
349-
int mid = (begin + end) >> 1;
350+
int mid = begin + ((end - begin) >> 1);
350351
int cmp = strncmp(istate->cache[mid]->name, prefix->buf, prefix->len);
351352
if (cmp == 0) /* mid has same prefix; look in second part */
352353
begin = mid + 1;

read-cache.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,7 @@ static int index_name_stage_pos(const struct index_state *istate, const char *na
549549
first = 0;
550550
last = istate->cache_nr;
551551
while (last > first) {
552-
int next = (last + first) >> 1;
552+
int next = first + ((last - first) >> 1);
553553
struct cache_entry *ce = istate->cache[next];
554554
int cmp = cache_name_stage_compare(name, namelen, stage, ce->name, ce_namelen(ce), ce_stage(ce));
555555
if (!cmp)

sh-i18n--envsubst.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ sorted_string_list_member (const string_list_ty *slp, const char *s)
249249
{
250250
/* Here we know that if s is in the list, it is at an index j
251251
with j1 <= j < j2. */
252-
size_t j = (j1 + j2) >> 1;
252+
size_t j = j1 + ((j2 - j1) >> 1);
253253
int result = strcmp (slp->item[j], s);
254254

255255
if (result > 0)

0 commit comments

Comments
 (0)