git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Allan Xavier <allan.x.xavier@oracle.com>
To: gitster@pobox.com
Cc: Allan Xavier <allan.x.xavier@oracle.com>, git@vger.kernel.org
Subject: [PATCH] line-log.c: prevent crash during union of too many ranges
Date: Thu,  2 Mar 2017 17:29:02 +0000	[thread overview]
Message-ID: <20170302172902.16850-1-allan.x.xavier@oracle.com> (raw)

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>
---

Originally sent to git-security@googlegroups.com to give hosted
services a chance to apply this if they were affected.

 line-log.c          | 15 +++++++--------
 t/t4211-line-log.sh | 10 ++++++++++
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/line-log.c b/line-log.c
index 65f3558b3..951029665 100644
--- a/line-log.c
+++ b/line-log.c
@@ -144,7 +144,7 @@ void sort_and_merge_range_set(struct range_set *rs)
 static void range_set_union(struct range_set *out,
 			     struct range_set *a, struct range_set *b)
 {
-	int i = 0, j = 0, o = 0;
+	int i = 0, j = 0;
 	struct range *ra = a->ranges;
 	struct range *rb = b->ranges;
 	/* 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,
 			new = &rb[j++];
 		if (new->start == new->end)
 			; /* empty range */
-		else if (!o || out->ranges[o-1].end < new->start) {
+		else if (!out->nr || out->ranges[out->nr-1].end < new->start) {
 			range_set_grow(out, 1);
-			out->ranges[o].start = new->start;
-			out->ranges[o].end = new->end;
-			o++;
-		} else if (out->ranges[o-1].end < new->end) {
-			out->ranges[o-1].end = new->end;
+			out->ranges[out->nr].start = new->start;
+			out->ranges[out->nr].end = new->end;
+			out->nr++;
+		} else if (out->ranges[out->nr-1].end < new->end) {
+			out->ranges[out->nr-1].end = new->end;
 		}
 	}
-	out->nr = o;
 }
 
 /*
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 9d87777b5..d0377fae5 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -106,4 +106,14 @@ test_expect_success '-L with --output' '
 	test_line_count = 70 log
 '
 
+test_expect_success 'range_set_union' '
+	test_seq 500 > c.c &&
+	git add c.c &&
+	git commit -m "many lines" &&
+	test_seq 1000 > c.c &&
+	git add c.c &&
+	git commit -m "modify many lines" &&
+	git log $(for x in $(test_seq 200); do echo -L $((2*x)),+1:c.c; done)
+'
+
 test_done
-- 
2.11.0


             reply	other threads:[~2017-03-02 23:38 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-02 17:29 Allan Xavier [this message]
2017-03-03 19:19 ` [PATCH] line-log.c: prevent crash during union of too many ranges Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170302172902.16850-1-allan.x.xavier@oracle.com \
    --to=allan.x.xavier@oracle.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).