Re: [PATCH] t9302: Protect against OS X normalization

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 9, 2010, 06:53 UTC
Message-ID: <7vocjyq4qq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100209060845.GD14736@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 5 quoted lines

> I am not 100% sure this will still trigger the failure that 8424981 was
> meant to fix. From my recollection of the bug, it not only needed an
> unterminated string (which we get by having a directory) but the string
> length and presence of multiple spread-out characters may have been
> relevant.

I think the test was triggering by chance. We give the directory name as pfx to write_name_quoted() which in turn tells quote_c_style_counted() to consume only pfxlen, but the random junk after pfxlen was being looked at because we didn't decrement maxlen.

The call chain that led to this part looks like this:

 tree.c:read_tree_recursive() ll.127-131
 - allocate newbase[], fill the leading directory path
 - call read_tree_recursive with newbase as base
   tree.c:read_tree_recursive() l.114
   - call callback function
     builtin-ls-tree.c:show_tree() l.114
     - call write_name_quotedpfx() with the given base
       unfixed code simply oversteps the end of newbase[] given from the
       toplevel read_tree_recursive!

I think there is no reliable reproduction recipe for this, as we don't control what garbage is in the tail of malloc'ed memory; valgrind would have found it, though.

Let's revert the test part of the patch.