[ JGIT ] incompatiblity found in DirCache

[ JGIT ] incompatiblity found in DirCache

[Adam W. Hawks]

When using the DirCache interface to the index you can create a invalid/corrupt tree for git 1.6.5.

The problem seems to be you can add a path to the index that starts with a "/" and DirCache creates a entry with a mode but no path.
This causes git 1.6.5 to fail with a corrupt tree.

The following code will create the problem

import java.io.File;
import java.io.IOException;
import java.util.Calendar;
import java.util.Date;
import java.util.TimeZone;

import org.spearce.jgit.dircache.DirCache;
import org.spearce.jgit.dircache.DirCacheBuilder;
import org.spearce.jgit.dircache.DirCacheEntry;
import org.spearce.jgit.lib.Commit;
import org.spearce.jgit.lib.FileMode;
import org.spearce.jgit.lib.ObjectId;
import org.spearce.jgit.lib.ObjectWriter;
import org.spearce.jgit.lib.PersonIdent;
import org.spearce.jgit.lib.RefUpdate;
import org.spearce.jgit.lib.Repository;
import org.spearce.jgit.lib.RefUpdate.Result;

public class BuildTest
{
	private Repository db;
	
	public static void main(String[] args)
	{
		BuildTest bt = new BuildTest();
		bt.doit();
	}
	
	public void doit()
	{		
		Date when = Calendar.getInstance().getTime();
		File gitDir = new File("gitProblem/.git");
		gitDir.mkdirs();
		try
		{
			db = new Repository(gitDir);
			db.create(true);
			DirCache dirc = DirCache.newInCore();
			DirCacheBuilder dcb = dirc.builder();
			byte[] data = "Some File data".getBytes();
			ObjectWriter ow = new ObjectWriter(db);
			ObjectId dataId = ow.writeBlob(data);
			DirCacheEntry newEntry = new DirCacheEntry("/someDir/someFile");
			newEntry.setAssumeValid(false);
			newEntry.setFileMode(FileMode.REGULAR_FILE);
			newEntry.setLastModified(when.getTime());
			newEntry.setLength(data.length);
			newEntry.setObjectId(dataId);
			dcb.add(newEntry );
			dcb.finish();
			dirc = dcb.getDirCache();
			PersonIdent pi = new PersonIdent("someonw","someone@somewhere",when,TimeZone.getDefault());
			ObjectId tree = dirc.writeTree(new ObjectWriter(db));
			Commit commit = new Commit(db);
			commit.setAuthor(pi);
			commit.setCommitter(pi);
			commit.setMessage("This causes a corrupt tree");
			commit.setTreeId(tree);
			commit.commit();
			ObjectId cid = commit.getCommitId();
			RefUpdate ru = db.updateRef("refs/heads/master");		
			ru.setExpectedOldObjectId(ObjectId.zeroId());
			ru.setNewObjectId(cid);
			ru.setRefLogIdent(pi);
			ru.setRefLogMessage("some reflog message", true);
			Result result = ru.update();
			System.out.println("Result = "+result.toString());
		}
		catch (IOException e)
		{
			System.out.println(e);
			e.printStackTrace();
			System.exit(1);
		}
	}	
}

Re: [ JGIT ] incompatiblity found in DirCache

[Robin Rosenberg]

onsdag 09 september 2009 20:55:39 skrev "Adam W. Hawks" <awhawks@writeme.com>:
> When using the DirCache interface to the index you can create a invalid/corrupt tree for git 1.6.5.
> 
> The problem seems to be you can add a path to the index that starts with a "/" and DirCache creates a entry with a mode but no path.
> This causes git 1.6.5 to fail with a corrupt tree.

I think there are more ways of entering bad stuff. Preventing a deliberate programmatic creation of invalid trees is probably not the most important
thing, but then again, validating the data to prevent e.g. the EGit plugin from doing it by mistake due to bugs could probably
be worthwhile.

-- robin

Re: [ JGIT ] incompatiblity found in DirCache

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
> onsdag 09 september 2009 20:55:39 skrev "Adam W. Hawks" <awhawks@writeme.com>:
> > When using the DirCache interface to the index you can create a
> > invalid/corrupt tree for git 1.6.5.
> > 
> > The problem seems to be you can add a path to the index that starts
> > with a "/" and DirCache creates a entry with a mode but no path.
> > This causes git 1.6.5 to fail with a corrupt tree.
> 
> I think there are more ways of entering bad stuff. Preventing a
> deliberate programmatic creation of invalid trees is probably not
> the most important thing, but then again, validating the data to
> prevent e.g. the EGit plugin from doing it by mistake due to bugs
> could probably be worthwhile.

We already check for and fail fast on a 0 mode in DirCache, as this
mode is also not valid in the index, or in a git tree.

We should be doing the same thing for an empty path name.  "a//b" is
not a valid path in the index, as "" is not a valid tree entry path.
For the same reason, "/a" is not a valid path in the index.

Unfortunately our API also allows you to try and create a name of
"a\u0000b", which is a valid Java string, but will create a corrupt
tree.  \u0000 in a name with more than 4095 bytes will also create
a corrupt index (shorter strings are semi-valid because shorter
strings use a Pascal like string format, and longer ones use a C
like string format).  Though C git is unable to access a path whose
name contains "\u0000", no matter how long the string is.

I think we should try a bit harder in DirCache to prevent these sorts
of really bad entries from being constructed by application code.
Yes, applications should not do this, but I think the library also
should not write known bogus trash to the disk and claim it is OK.

I'll try to work up a patch for this today.

-- 
Shawn.