Re: [PATCH v10] credential-store: ignore bogus lines from store file

From: Jeff King <peff@peff.net>
Date: May 3, 2020, 10:06 UTC
Message-ID: <20200503100627.GA171296@coredump.intra.peff.net>
In-Reply-To: <xmqqo8r6jc8i.fsf@gitster.c.googlers.com>

On Sat, May 02, 2020 at 01:47:09PM -0700, Junio C Hamano wrote:

Show 8 quoted lines

> > As a special case, flag files with CRLF endings as invalid early
> > to prevent current problems in credential_from_url_gently() with
> > handling of '\r' in the host.
> 
> I do not think it hurts to silently ignore a line that ends with CR,
> but only because I do not think credential_from_url_gently() would
> not match such a line when asked to match something without
> complaining.

I wondered if we might hit a case where the CR ends up in the path, like:

  $ printf 'https://user:pass@example.com/\r\n' >creds
  $ echo url=https://example.com/ |
    git credential-store --file=creds get
  username=user
  password=pass

because the path is parsed as missing in the incoming pattern (and thus we match any path, even "\r").

But credential-store would never write such a path in the first place. Even with the trailing slash on an incoming URL, it will write:

  https://example.com

without a slash at all (and thus any inserted CR would be part of the hostname). So somebody would have to have inserted it themselves, or have turned useHTTPPath on (in which case we _would_ complain on the matching side, too, because we'd try matching the path with a CR in it).

I think it's reasonable to assume that any CR would have been a problem even in older versions.

-Peff