[PATCH 05/16] apply: use skip_prefix instead of raw addition

From: Jeff King <peff@peff.net>
Date: Jun 18, 2014, 19:45 UTC
Message-ID: <20140618194534.GE22622@sigill.intra.peff.net>
In-Reply-To: <20140618194117.GA22269@sigill.intra.peff.net>

A submodule diff generally has content like:

  -Subproject commit [0-9a-f]{40}
  +Subproject commit [0-9a-f]{40}

When we are using "git apply --index" with a submodule, we first apply the textual diff, and then parse that result to figure out the new sha1.

If the diff has bogus input like:

  -Subproject commit 1234567890123456789012345678901234567890
  +bogus

we will parse the "bogus" portion. Our parser assumes that the buffer starts with "Subproject commit", and blindly skips past it using strlen(). This can cause us to read random memory after the buffer.

This problem was unlikely to have come up in practice (since it requires a malformed diff), and even when it did, we likely noticed the problem anyway as the next operation was to call get_sha1_hex on the random memory.

However, we can easily fix it by using skip_prefix to notice the parsing error.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/apply.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 9c5724e..bc924ab 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3847,9 +3847,10 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned
 	ce->ce_flags = create_ce_flags(0);
 	ce->ce_namelen = namelen;
 	if (S_ISGITLINK(mode)) {
-		const char *s = buf;
+		const char *s;
 
-		if (get_sha1_hex(s + strlen("Subproject commit "), ce->sha1))
+		if (!skip_prefix(buf, "Subproject commit ", &s) ||
+		    get_sha1_hex(s, ce->sha1))
 			die(_("corrupt patch for submodule %s"), path);
 	} else {
 		if (!cached) {

-- 
2.0.0.566.gfe3e6b2