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

2017-09-27 Thread Junio C Hamano
Jeff King writes: > I had the same concern, but actually truncation is not a problem here > (for a symlink or a symref). We are only seeing if the contents look > vaguely correct, so really we never parse past "refs/" in either case. Ahh, OK. It seems that at least for this week, my reviews alw

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

2017-09-27 Thread Jeff King
On Wed, Sep 27, 2017 at 04:06:22PM +0900, Junio C Hamano wrote: > A few tangents I noticed: > > - the result of readlink should be checked with starts_with() in >the modern codebase (#leftoverbits). Yes, though it needs to NUL-terminate, too (readlink does not do so automatically). Again, w

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

2017-09-27 Thread Junio C Hamano
Jeff King writes: > 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) > +

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

2017-09-26 Thread Jeff King
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 go