On Thu, Aug 28, 2014 at 01:47:52PM -0700, Junio C Hamano wrote: > Johannes Schindelin <johannes.schindelin@xxxxxx> writes: > > > When fsck'ing an incoming pack, we need to fsck objects that cannot be > > read via read_sha1_file() because they are not local yet (and might even > > be rejected if transfer.fsckobjects is set to 'true'). > > > > For commits, there is a hack in place: we basically cache commit > > objects' buffers anyway, but the same is not true, say, for tag objects. > > > > By refactoring fsck_object() to take the object buffer and size as > > optional arguments -- optional, because we still fall back to the > > previous method to look at the cached commit objects if the caller > > passes NULL -- we prepare the machinery for the upcoming handling of tag > > objects. > > > > The assumption that such buffers are inherently NUL terminated is now > > wrong, of course, hence we pass the size of the buffer so that we can > > add a sanity check later, to prevent running past the end of the buffer. > > A nice side effect may be that we can now check (and perhaps warn) a > commit buffer with a NUL inside, perhaps? I am not suggesting to > add such a check to this series, but mentioning the possibilty here > may have a merit. I think that is a good check to add at some point. I demonstrated quite a while ago that you can get up to some mischief by "hiding" bytes after a NUL in the commit message. If we ever see feasible collision attacks against sha1, this is going to be an obvious vector for hiding random bytes to achieve the collisions. The downside is that git's data model in theory promises to store arbitrary bytes in people's commit messages. IMHO we have gotten far enough from that in practice that I do not think anybody is seriously doing it (you could not really use most of the history inspection tools in a reasonable way; you would have to write a parallel set of tools that never shows log messages, and then extracts and does something separate with the commit content). -Peff -- 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