[PATCH 1/3] validate_headref: NUL-terminate HEAD buffer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



When we are checking to see if we have a git repo, we peek
into the HEAD file and see if it's a plausible symlink,
symref, or detached HEAD.

For the latter two, we read the contents with read_in_full(),
which means they aren't NUL-terminated. The symref check is
careful to respect the length we got, but the sha1 check
will happily parse up to 40 bytes, even if we read fewer.

E.g.,:

  echo 1234 >.git/HEAD
  git rev-parse

will parse 36 uninitialized bytes from our stack buffer.

This isn't a big deal in practice. Our buffer is 256 bytes,
so we know we'll never read outside of it. The worst case is
that the uninitialized bytes look like valid hex, and we
claim a bogus HEAD file is valid. The chances of this
happening randomly are quite slim, but let's be careful.

One option would be to check that "len == 41" before feeding
the buffer to get_sha1_hex(). But we'd like to eventually
prepare for a world with variable-length hashes. Let's
NUL-terminate as soon as we've read the buffer (we already
even leave a spare byte to do so!). That fixes this problem
without depending on the size of an object id.

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
 path.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/path.c b/path.c
index b533ec938d..3e4d7505ef 100644
--- a/path.c
+++ b/path.c
@@ -662,6 +662,10 @@ int validate_headref(const char *path)
 	len = read_in_full(fd, buffer, sizeof(buffer)-1);
 	close(fd);
 
+	if (len < 0)
+		return -1;
+	buffer[len] = '\0';
+
 	/*
 	 * Is it a symbolic ref?
 	 */
-- 
2.14.2.988.g01c8b37dde




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux