From 101d132c0703f06bfa6ecc1e6928b3d718db3a11 Mon Sep 17 00:00:00 2001
From: "Shawn O. Pearce" <spearce@spearce.org>
Date: Mon, 29 Sep 2008 20:54:47 -0700
Subject: [PATCH] index-pack: Detect SHA-1 hash collisions to avoid replacing
 objects

When indexing a pack file coming in from the network there may
be an object contained in the pack that we already have.  Its
only safe to retain that object in our newly stored pack if it
exactly matches the content we already have on disk.  Storing
a different content for the same object name runs the risk of
allowing a malicious attacker to replace an object in our store.

Data validation is performed the first time we have the object
in memory, which occurs when we first compute its SHA-1.  This
is the earliest opportunity we have to validate that there is
no collision, and it reduces the number of times we need to do
a read for an object.  However our memory usage when we process
a whole object is now increased as the collision test is done
by loading the entire object into memory.  Future improvements
may permit a streaming compare of the objects.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../src/org/spearce/jgit/transport/IndexPack.java  | 63 ++++++++++++++++++----
 1 file changed, 54 insertions(+), 9 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index bc528966..8a66ec97 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -375,6 +375,7 @@ public class IndexPack {
 			objectDigest.update(data);
 			tempObjectId.fromRaw(objectDigest.digest(), 0);
 
+			verifyNoCollision(type, data);
 			oe = new PackedObjectInfo(pos, crc32, tempObjectId);
 			entries[entryCount++] = oe;
 		}
@@ -622,7 +623,7 @@ public class IndexPack {
 				r = new ArrayList<UnresolvedDelta>(8);
 				baseByPos.put(base, r);
 			}
-			inflateFromInput(false);
+			skipInflateFromInput(sz);
 			r.add(new UnresolvedDelta(pos, (int) crc.getValue()));
 			deltaCount++;
 			break;
@@ -637,7 +638,7 @@ public class IndexPack {
 				r = new ArrayList<UnresolvedDelta>(8);
 				baseById.put(base, r);
 			}
-			inflateFromInput(false);
+			skipInflateFromInput(sz);
 			r.add(new UnresolvedDelta(pos, (int) crc.getValue()));
 			deltaCount++;
 			break;
@@ -649,17 +650,30 @@ public class IndexPack {
 
 	private void whole(final int type, final long pos, final long sz)
 			throws IOException {
+		final byte[] data = inflateFromInput(sz);
 		objectDigest.update(Constants.encodedTypeString(type));
 		objectDigest.update((byte) ' ');
 		objectDigest.update(Constants.encodeASCII(sz));
 		objectDigest.update((byte) 0);
-		inflateFromInput(true);
+		objectDigest.update(data);
 		tempObjectId.fromRaw(objectDigest.digest(), 0);
 
+		verifyNoCollision(type, data);
 		final int crc32 = (int) crc.getValue();
 		entries[entryCount++] = new PackedObjectInfo(pos, crc32, tempObjectId);
 	}
 
+	private void verifyNoCollision(final int type, final byte[] data)
+			throws IOException {
+		final ObjectLoader ldr = repo.openObject(tempObjectId);
+		if (ldr != null) {
+			final byte[] existingData = ldr.getCachedBytes();
+			if (ldr.getType() != type || !Arrays.equals(data, existingData)) {
+				throw new IOException("collision in " + tempObjectId.name());
+			}
+		}
+	}
+
 	// Current position of {@link #bOffset} within the entire file.
 	private long position() {
 		return bBase + bOffset;
@@ -747,7 +761,7 @@ public class IndexPack {
 		bOffset = 0;
 	}
 
-	private void inflateFromInput(final boolean digest) throws IOException {
+	private void skipInflateFromInput(long sz) throws IOException {
 		final Inflater inf = inflater;
 		try {
 			final byte[] dst = objectData;
@@ -765,21 +779,52 @@ public class IndexPack {
 
 				int free = dst.length - n;
 				if (free < 8) {
-					if (digest)
-						objectDigest.update(dst, 0, n);
+					sz -= n;
 					n = 0;
 					free = dst.length;
 				}
-
 				n += inf.inflate(dst, n, free);
 			}
-			if (digest)
-				objectDigest.update(dst, 0, n);
+			if (n != sz)
+				throw new DataFormatException("wrong decompressed length");
+			n = bAvail - inf.getRemaining();
+			if (n > 0) {
+				crc.update(buf, p, n);
+				use(n);
+			}
+		} catch (DataFormatException dfe) {
+			throw corrupt(dfe);
+		} finally {
+			inf.reset();
+		}
+	}
+
+	private byte[] inflateFromInput(final long sz) throws IOException {
+		final byte[] dst = new byte[(int) sz];
+		final Inflater inf = inflater;
+		try {
+			int n = 0;
+			int p = -1;
+			while (!inf.finished()) {
+				if (inf.needsInput()) {
+					if (p >= 0) {
+						crc.update(buf, p, bAvail);
+						use(bAvail);
+					}
+					p = fillFromInput(1);
+					inf.setInput(buf, p, bAvail);
+				}
+
+				n += inf.inflate(dst, n, dst.length - n);
+			}
+			if (n != sz)
+				throw new DataFormatException("wrong decompressed length");
 			n = bAvail - inf.getRemaining();
 			if (n > 0) {
 				crc.update(buf, p, n);
 				use(n);
 			}
+			return dst;
 		} catch (DataFormatException dfe) {
 			throw corrupt(dfe);
 		} finally {
-- 
2.11.4.GIT