Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer

2013-12-18 Thread Jeff King
On Wed, Dec 18, 2013 at 12:15:19PM -0800, Junio C Hamano wrote: > > Thanks. Are you picking this up with a commit message, or did you want > > me to re-send with the usual message/signoff? > > I think this should be sufficient ;-) > > -- >8 -- > From: Jeff King > Date: Tue, 17 Dec 2013 18:22:31

Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer

2013-12-18 Thread Junio C Hamano
Jeff King writes: > On Wed, Dec 18, 2013 at 12:07:02PM -0800, Junio C Hamano wrote: > >> Jeff King writes: >> >> >> > + prune_object(path->buf, sha1); >> >> > + path->len = baselen; >> >> >> >> This is minor, but I prefer using strbuf_setlen() for th

Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer

2013-12-18 Thread Jeff King
On Wed, Dec 18, 2013 at 12:07:02PM -0800, Junio C Hamano wrote: > Jeff King writes: > > >> > +prune_object(path->buf, sha1); > >> > +path->len = baselen; > >> > >> This is minor, but I prefer using strbuf_setlen() for this. > > > > Good catch. I d

Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer

2013-12-18 Thread Junio C Hamano
Jeff King writes: >> > + prune_object(path->buf, sha1); >> > + path->len = baselen; >> >> This is minor, but I prefer using strbuf_setlen() for this. > > Good catch. I do not think it is minor at all; my version is buggy. > After the loop ends, path->len does no

Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer

2013-12-18 Thread Jeff King
On Wed, Dec 18, 2013 at 11:35:34AM -0800, Junio C Hamano wrote: > This function is called to remove > > * Any tmp_* found directly in .git/objects/ > * Any tmp_* found directly in .git/objects/pack/ > * Any tmp_obj_* found directly in .git/objects/??/ > > and shares the same expiration logic

Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer

2013-12-18 Thread Junio C Hamano
Jeff King writes: > Converting it to use strbuf looks like it will actually let us drop a > bunch of copying, too, as we just end up in mkpath at the very lowest > level. I.e., something like below. Thanks; I may have a few minor comments, but overall, I like the placement of mkpath() in the res

Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer

2013-12-17 Thread Jeff King
On Tue, Dec 17, 2013 at 10:51:30AM -0800, Junio C Hamano wrote: > Michael Haggerty writes: > > > Dimension the buffer based on PATH_MAX rather than a magic number, and > > verify that the path fits in it before continuing. > > > > Signed-off-by: Michael Haggerty > > --- > > > > I don't think th

Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer

2013-12-17 Thread Junio C Hamano
Michael Haggerty writes: > Dimension the buffer based on PATH_MAX rather than a magic number, and > verify that the path fits in it before continuing. > > Signed-off-by: Michael Haggerty > --- > > I don't think that this problem is remotely exploitable, because the > size of the string doesn't d

Re: [PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer

2013-12-17 Thread Antoine Pelisse
On Tue, Dec 17, 2013 at 2:43 PM, Michael Haggerty wrote: > Dimension the buffer based on PATH_MAX rather than a magic number, and > verify that the path fits in it before continuing. > > Signed-off-by: Michael Haggerty > --- > > I don't think that this problem is remotely exploitable, because the

[PATCH 2/3] prune_object_dir(): verify that path fits in the temporary buffer

2013-12-17 Thread Michael Haggerty
Dimension the buffer based on PATH_MAX rather than a magic number, and verify that the path fits in it before continuing. Signed-off-by: Michael Haggerty --- I don't think that this problem is remotely exploitable, because the size of the string doesn't depend on inputs that can be influenced by