Re: Possible bug reports for git-credential-libsecret

From: <>
Date: Mar 18, 2022, 18:19 UTC
Message-ID: <CAPb83E-9=dLoc7ywHjw58Boxp9wkPaAXFy+UH+HJUE9tUzw=_A@mail.gmail.com>
In-Reply-To: <xmqqtubv9n1i.fsf@gitster.g>

Hi.

I previously determined that code can cause unexpected behavior because:
  (1) the vulnerable code of "credential.c" patched in CVE-2020-5260
(vulnerability related to newline characters) identically exists in
the "git-credential-libsecret.c" file, and
  (2) the test code (please see below) that should be failed, which
was added in CVE-2020-5260 patch, did not be failed in
git-credential-libsecret.

--- TEST CODE for CVE-2020-5260 ---
test_expect_success 'url parser rejects embedded newlines' '
               test_must_fail git credential fill <<-\EOF
               url=https://one.example.com?%0ahost=two.example.com/
               EOF
---------------------------------------------------

However, as you said, if the data has already been purified from credential_write_item and then flows to credential-libsecret, it doesn't seem like a big deal. Thank you for your response.

Best regards, Seunghoon Woo

2022년 3월 19일 (토) 오전 2:40, Junio C Hamano <gitster@pobox.com>님이 작성:

Show 29 quoted lines

>
> "‍우승훈[ 대학원석·박사통합과정수료연구(재학) / 컴퓨터학과 ]"
> <seunghoonwoo@korea.ac.kr> writes:
>
> > Recently, I noted that the vulnerability patch (CVE-2020-5260) for
> > "git credential" is not applied in the "git-credential-libsecret"
> > program (./contrib/credential/libsecret/git-credential-libsecret.c).
>
> The code in credential.c touches the data directly obtained from the
> outside world.  It opens a pipe to backends like credential-store
> and credential-libsecret and feeds the data that is cleansed by the
> code you saw in credential.c::credential_write_item() to these
> backends.
>
> What these backends write come from these keyring storage systems.
> If you look at other backends in contrib/ and also credential-store
> and credential-cache, you'll see similar code to give "username=%s"
> and "password=%s" back to us in all of them.
>
> Do you have a reason to suspect that these keyring storage systems
> the backends talk with can be as malicious as the data source the
> CVE patch tried to protect us against?  If an attacker can already
> contaminate the contents of the keyrings these backends read from
> and write to, they do not have to use "git credential" as their
> attack vector, I would suspect.
>
> So I would think it is natural to expect that the patch you saw is
> the only defence you need.
>

-- 
Best regards,


Seunghoon Woo

Korea University Dept. of Computer Science and Engineering

Computer & Communication Security Lab.

seunghoonwoo@korea.ac.kr

(+82)10-8147-9308