@matheusbsbs / matheus-bagliano / commits / aaae0bf787

line-log.c: prevent crash during union of too many ranges

The existing implementation of range_set_union does not correctly reallocate memory, leading to a heap overflow when it attempts to union more than 24 separate line ranges. For struct range_set *out to grow correctly it must have out->nr set to the current size of the buffer when it is passed to range_set_grow. However, the existing implementation of range_set_union only updates out->nr at the end of the function, meaning that it is always zero before this. This results in range_set_grow never growing the buffer, as well as some of the union logic itself being incorrect as !out->nr is always true. The reason why 24 is the limit is that the first allocation of size 1 ends up allocating a buffer of size 24 (due to the call to alloc_nr in ALLOC_GROW). This goes some way to explain why this hasn't been caught before. Fix the problem by correctly updating out->nr after reallocating the range_set. As this results in out->nr containing the same value as the variable o, replace o with out->nr as well. Finally, add a new test to help prevent the problem reoccurring in the future. Thanks to Vegard Nossum for writing the test. Signed-off-by: Allan Xavier <allan.x.xavier@oracle.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Allan Xavier committed Mar 2, 2017 at 17:29 UTC aaae0bf787f09ba102f69c3cf85d37e6554ab9fd

2 files changed +17 -8

line-log.c

+7 -8

@@ -144,7 +144,7 @@ void sort_and_merge_range_set(struct range_set *rs)

144 static void range_set_union(struct range_set *out,

145 struct range_set *a, struct range_set *b)

146 {

147 - int i = 0, j = 0, o = 0;

147 + int i = 0, j = 0;

148 struct range *ra = a->ranges;

149 struct range *rb = b->ranges;

150 /* cannot make an alias of out->ranges: it may change during grow */

@@ -167,16 +167,15 @@ static void range_set_union(struct range_set *out,

167 new = &rb[j++];

168 if (new->start == new->end)

169 ; /* empty range */

170 - else if (!o || out->ranges[o-1].end < new->start) {

170 + else if (!out->nr || out->ranges[out->nr-1].end < new->start) {

171 range_set_grow(out, 1);

172 - out->ranges[o].start = new->start;

173 - out->ranges[o].end = new->end;

174 - o++;

175 - } else if (out->ranges[o-1].end < new->end) {

176 - out->ranges[o-1].end = new->end;

172 + out->ranges[out->nr].start = new->start;

173 + out->ranges[out->nr].end = new->end;

174 + out->nr++;

175 + } else if (out->ranges[out->nr-1].end < new->end) {

176 + out->ranges[out->nr-1].end = new->end;

177 }

178 }

179 - out->nr = o;

179 }

180

181 /*

t/t4211-line-log.sh

+10

@@ -106,4 +106,14 @@ test_expect_success '-L with --output' '

106 test_line_count = 70 log

107 '

108

109 +test_expect_success 'range_set_union' '

110 + test_seq 500 > c.c &&

111 + git add c.c &&

112 + git commit -m "many lines" &&

113 + test_seq 1000 > c.c &&

114 + git add c.c &&

115 + git commit -m "modify many lines" &&

116 + git log $(for x in $(test_seq 200); do echo -L $((2*x)),+1:c.c; done)

117 +'

118 +

119 test_done