On Fri, May 22, 2015 at 2:12 PM, Jeff King <peff@xxxxxxxx> wrote: > So I think there are two possibilities for improving this: > > 1. Find places where we expect the object will not exist (like the > collision_test check you pointed out) and use a > "has_sha1_file_fast" that accepts that it may very occasionally > erroneously return false. In this case it would mean potentially > skipping a collision check, but I think that is OK. That could have > security implications, but only if an attacker: > > a. has broken sha1 to generate a colliding object > > b. can manipulate the victim into repacking in a loop > > c. can manipulate the victim into fetching (or receiving a push) > simultaneously with (b) > > at which point they can try to race the repack procedure to add > their colliding object to the repository. It seems rather unlikely > (especially part a). In case you want to back away from option 2 because it starts to leak raciness, which your old commit tried to fix in the first place. I think the only other place that tests for lots of non-existent loose objects is write_sha1_file (e.g. "tar -xf bigtarball.tar.gz; cd bigtarball; git init; git add ."). But the number of calls should be much smaller compared to index-pack and it does not use has_sha1_file, it uses check_and_freshen_file() instead. There are other places where has_sha1_file() may return 0, but I think the number of calls is even smaller to bother (shallow.c, fetch-pack.c, apply.c, buik-checkin.c) -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html