{"thread":{"id":"57583","subject":"Possible bug reports for git-credential-libsecret","startedAt":"2022-03-18T17:13:46Z","lastAt":"2022-03-18T18:17:37Z","messageCount":3,"participants":["Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"451635","messageId":"CAPb83E-Y8grG7MVWqjqPH6SsdZ2JJ62XUeQAe=Wb_zgn0RL90A@mail.gmail.com","threadId":"57583","inReplyTo":null,"subject":"Possible bug reports for git-credential-libsecret","fromName":"","fromEmail":"","sentAt":"2022-03-18T17:15:30Z","receivedAt":"2022-03-18T17:13:46Z","isPatch":false,"sender":null,"body":"Dear Git Development team\n\nHi.\nRecently, I noted that the vulnerability patch (CVE-2020-5260) for\n\"git credential\" is not applied in the \"git-credential-libsecret\"\nprogram (./contrib/credential/libsecret/git-credential-libsecret.c).\n\nActually, I'm not sure whether \"git-credential-libsecret\" is actually\nused. But it can be potentially intimidating, thus it would be good to\napply the same patch of \"git credential\" to\n\"git-credential-libsecret\".\n\n- Patch for CVE-2020-5260\n   * https://github.com/git/git/commit/9a6bbee8006c24b46a85d29e7b38cfa79e9ab21b\n\n- Similar but not patched code in \"git-credential-libsecret\"\n   * https://github.com/git/git/blob/master/contrib/credential/libsecret/git-credential-libsecret.c#L306-L311\n   * When I put the malicious URL in the test code shown in the\nCVE-2020-5260 patch into \"git-credential-libsecret\", it does not\nterminate.\n\nCould you please check whether this is an actual bug?\n\nThank you.\nBest regards,\nSeunghoon Woo\n\n-- \n\nBest regards,\nSeunghoon Woo\nKorea University Dept. of Computer Science and Engineering\nComputer & Communication Security Lab.\nseunghoonwoo@korea.ac.kr\n(+82)10-8147-9308\n"},{"id":"451637","messageId":"xmqqtubv9n1i.fsf@gitster.g","threadId":"57583","inReplyTo":"CAPb83E-Y8grG7MVWqjqPH6SsdZ2JJ62XUeQAe=Wb_zgn0RL90A@mail.gmail.com","subject":"Re: Possible bug reports for git-credential-libsecret","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-18T17:40:41Z","receivedAt":"2022-03-18T17:40:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"‍우승훈[ 대학원석·박사통합과정수료연구(재학) / 컴퓨터학과 ]\"\n<seunghoonwoo@korea.ac.kr> writes:\n\n> Recently, I noted that the vulnerability patch (CVE-2020-5260) for\n> \"git credential\" is not applied in the \"git-credential-libsecret\"\n> program (./contrib/credential/libsecret/git-credential-libsecret.c).\n\nThe code in credential.c touches the data directly obtained from the\noutside world.  It opens a pipe to backends like credential-store\nand credential-libsecret and feeds the data that is cleansed by the\ncode you saw in credential.c::credential_write_item() to these\nbackends.\n\nWhat these backends write come from these keyring storage systems.\nIf you look at other backends in contrib/ and also credential-store\nand credential-cache, you'll see similar code to give \"username=%s\"\nand \"password=%s\" back to us in all of them.\n\nDo you have a reason to suspect that these keyring storage systems\nthe backends talk with can be as malicious as the data source the\nCVE patch tried to protect us against?  If an attacker can already\ncontaminate the contents of the keyrings these backends read from\nand write to, they do not have to use \"git credential\" as their\nattack vector, I would suspect.\n\nSo I would think it is natural to expect that the patch you saw is\nthe only defence you need.\n\n"},{"id":"451638","messageId":"CAPb83E-9=dLoc7ywHjw58Boxp9wkPaAXFy+UH+HJUE9tUzw=_A@mail.gmail.com","threadId":"57583","inReplyTo":"xmqqtubv9n1i.fsf@gitster.g","subject":"Re: Possible bug reports for git-credential-libsecret","fromName":"","fromEmail":"","sentAt":"2022-03-18T18:19:24Z","receivedAt":"2022-03-18T18:17:37Z","isPatch":false,"sender":null,"body":"Hi.\n\nI previously determined that code can cause unexpected behavior because:\n  (1) the vulnerable code of \"credential.c\" patched in CVE-2020-5260\n(vulnerability related to newline characters) identically exists in\nthe \"git-credential-libsecret.c\" file, and\n  (2) the test code (please see below) that should be failed, which\nwas added in CVE-2020-5260 patch, did not be failed in\ngit-credential-libsecret.\n\n--- TEST CODE for CVE-2020-5260 ---\ntest_expect_success 'url parser rejects embedded newlines' '\n               test_must_fail git credential fill <<-\\EOF\n               url=https://one.example.com?%0ahost=two.example.com/\n               EOF\n---------------------------------------------------\n\nHowever, as you said, if the data has already been purified from\ncredential_write_item and then flows to credential-libsecret, it\ndoesn't seem like a big deal.\nThank you for your response.\n\nBest regards,\nSeunghoon Woo\n\n2022년 3월 19일 (토) 오전 2:40, Junio C Hamano <gitster@pobox.com>님이 작성:\n>\n> \"‍우승훈[ 대학원석·박사통합과정수료연구(재학) / 컴퓨터학과 ]\"\n> <seunghoonwoo@korea.ac.kr> writes:\n>\n> > Recently, I noted that the vulnerability patch (CVE-2020-5260) for\n> > \"git credential\" is not applied in the \"git-credential-libsecret\"\n> > program (./contrib/credential/libsecret/git-credential-libsecret.c).\n>\n> The code in credential.c touches the data directly obtained from the\n> outside world.  It opens a pipe to backends like credential-store\n> and credential-libsecret and feeds the data that is cleansed by the\n> code you saw in credential.c::credential_write_item() to these\n> backends.\n>\n> What these backends write come from these keyring storage systems.\n> If you look at other backends in contrib/ and also credential-store\n> and credential-cache, you'll see similar code to give \"username=%s\"\n> and \"password=%s\" back to us in all of them.\n>\n> Do you have a reason to suspect that these keyring storage systems\n> the backends talk with can be as malicious as the data source the\n> CVE patch tried to protect us against?  If an attacker can already\n> contaminate the contents of the keyrings these backends read from\n> and write to, they do not have to use \"git credential\" as their\n> attack vector, I would suspect.\n>\n> So I would think it is natural to expect that the patch you saw is\n> the only defence you need.\n>\n\n\n-- \n\nBest regards,\n\n\nSeunghoon Woo\n\nKorea University Dept. of Computer Science and Engineering\n\nComputer & Communication Security Lab.\n\nseunghoonwoo@korea.ac.kr\n\n(+82)10-8147-9308\n"}]}