On Fri, Nov 26, 2021 at 12:45 PM Steve Sakoman <[email protected]> wrote: > > On Thu, Nov 25, 2021 at 12:49 AM Minjae Kim <[email protected]> wrote: > > > > git_connect_git in connect.c in Git before 2.30.1 allows a repository path > > to contain a newline character, > > which may result in unexpected cross-protocol requests, > > as demonstrated by the git://localhost:1234/%0d%0a%0d%0aGET%20/%20HTTP/1.1 > > substring. > > > > Upstream-Status: Backport > > [https://github.com/git/git/commit/a02ea577174ab8ed18f847cf1693f213e0b9c473] > > CVE: CVE-2021-40330 > > Signed-off-by: Minjae Kim <[email protected]> > > --- > > .../git/files/CVE-2021-40330.patch | 108 ++++++++++++++++++ > > This patch file will not apply: > > ERROR: git-2.24.3-r0 do_patch: Applying patch 'CVE-2021-40330.patch' > on target directory > '/home/steve/builds/poky-contrib/build/tmp/work/core2-64-poky-linux/git/2.24.3-r0/git-2.24.3' > Command Error: 'quilt --quiltrc > /home/steve/builds/poky-contrib/build/tmp/work/core2-64-poky-linux/git/2.24.3-r0/recipe-sysroot-native/etc/quiltrc > push' exited with 0 Output: > Applying patch CVE-2021-40330.patch > patching file connect.c > Hunk #1 FAILED at 1064. > 1 out of 1 hunk FAILED -- rejects in file connect.c > patching file t/t5570-git-daemon.sh > Hunk #1 succeeded at 103 with fuzz 1. > Patch CVE-2021-40330.patch does not apply (enforce with -f) > ERROR: Logfile of failure stored in: > /home/steve/builds/poky-contrib/build/tmp/work/core2-64-poky-linux/git/2.24.3-r0/temp/log.do_patch.298303 > ERROR: Task > (/home/steve/builds/poky-contrib/meta/recipes-devtools/git/git_2.24.3.bb:do_patch) > failed with exit code '1' > > Taking a closer look at this, it seems there are two issues. The > first is that your mailer (?) is changing tabs to spaces, and this is > why the patch won't apply. > > There is a second more minor issue -- there is a fuzz of 1 in the second hunk. > > I can fix these issues if you like, but it might be good for you to > track down why this is happening so it isn't an issue with future > patches you submit to the list. > > Let me know what you prefer!
I decided to go ahead and do the fixups so that I can get this patch into my next review set. So no need for you to do anything at this time. Steve > > meta/recipes-devtools/git/git.inc | 4 +- > > 2 files changed, 111 insertions(+), 1 deletion(-) > > create mode 100644 meta/recipes-devtools/git/files/CVE-2021-40330.patch > > > > diff --git a/meta/recipes-devtools/git/files/CVE-2021-40330.patch > > b/meta/recipes-devtools/git/files/CVE-2021-40330.patch > > new file mode 100644 > > index 0000000000..282cd3fbe5 > > --- /dev/null > > +++ b/meta/recipes-devtools/git/files/CVE-2021-40330.patch > > @@ -0,0 +1,108 @@ > > +From e77ca0c7d577408878d2b3e8c7336e6119cb3931 Mon Sep 17 00:00:00 2001 > > +From: Minjae Kim <[email protected]> > > +Date: Thu, 25 Nov 2021 06:36:26 +0000 > > +Subject: [PATCH] git_connect_git(): forbid newlines in host and path > > + > > +When we connect to a git:// server, we send an initial request that > > +looks something like: > > + > > + 002dgit-upload-pack repo.git\0host=example.com > > + > > +If the repo path contains a newline, then it's included literally, and > > +we get: > > + > > + 002egit-upload-pack repo > > + .git\0host=example.com > > + > > +This works fine if you really do have a newline in your repository name; > > +the server side uses the pktline framing to parse the string, not > > +newlines. However, there are many _other_ protocols in the wild that do > > +parse on newlines, such as HTTP. So a carefully constructed git:// URL > > +can actually turn into a valid HTTP request. For example: > > + > > + git://localhost:1234/%0d%0a%0d%0aGET%20/%20HTTP/1.1 > > %0d%0aHost:localhost%0d%0a%0d%0a > > + > > +becomes: > > + > > + 0050git-upload-pack / > > + GET / HTTP/1.1 > > + Host:localhost > > + > > + host=localhost:1234 > > + > > +on the wire. Again, this isn't a problem for a real Git server, but it > > +does mean that feeding a malicious URL to Git (e.g., through a > > +submodule) can cause it to make unexpected cross-protocol requests. > > +Since repository names with newlines are presumably quite rare (and > > +indeed, we already disallow them in git-over-http), let's just disallow > > +them over this protocol. > > + > > +Hostnames could likewise inject a newline, but this is unlikely a > > +problem in practice; we'd try resolving the hostname with a newline in > > +it, which wouldn't work. Still, it doesn't hurt to err on the side of > > +caution there, since we would not expect them to work in the first > > +place. > > + > > +The ssh and local code paths are unaffected by this patch. In both cases > > +we're trying to run upload-pack via a shell, and will quote the newline > > +so that it makes it intact. An attacker can point an ssh url at an > > +arbitrary port, of course, but unless there's an actual ssh server > > +there, we'd never get as far as sending our shell command anyway. We > > +_could_ similarly restrict newlines in those protocols out of caution, > > +but there seems little benefit to doing so. > > + > > +The new test here is run alongside the git-daemon tests, which cover the > > +same protocol, but it shouldn't actually contact the daemon at all. In > > +theory we could make the test more robust by setting up an actual > > +repository with a newline in it (so that our clone would succeed if our > > +new check didn't kick in). But a repo directory with newline in it is > > +likely not portable across all filesystems. Likewise, we could check > > +git-daemon's log that it was not contacted at all, but we do not > > +currently record the log (and anyway, it would make the test racy with > > +the daemon's log write). We'll just check the client-side stderr to make > > +sure we hit the expected code path. > > + > > +Reported-by: Harold Kim <[email protected]> > > +Signed-off-by: Jeff King <[email protected]> > > +Signed-off-by: Junio C Hamano <[email protected]> > > + > > +Upstream-Status: Backported > > [https://github.com/git/git/commit/a02ea577174ab8ed18f847cf1693f213e0b9c473] > > +CVE: CVE-2021-40330 > > +Signed-off-by: Minjae Kim <[email protected]> > > +--- > > + connect.c | 2 ++ > > + t/t5570-git-daemon.sh | 5 +++++ > > + 2 files changed, 7 insertions(+) > > + > > +diff --git a/connect.c b/connect.c > > +index b6451ab..929de9a 100644 > > +--- a/connect.c > > ++++ b/connect.c > > +@@ -1064,6 +1064,8 @@ static struct child_process *git_connect_git(int > > fd[2], char *hostandport, > > + target_host = xstrdup(hostandport); > > + > > + transport_check_allowed("git"); > > ++ if (strchr(target_host, '\n') || strchr(path, '\n')) > > ++ die(_("newline is forbidden in git:// hosts and repo > > paths")); > > + > > + /* > > + * These underlying connection commands die() if they > > +diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh > > +index 34487bb..79cd218 100755 > > +--- a/t/t5570-git-daemon.sh > > ++++ b/t/t5570-git-daemon.sh > > +@@ -103,6 +103,11 @@ test_expect_success 'fetch notices corrupt idx' ' > > + ) > > + ' > > + > > ++test_expect_success 'client refuses to ask for repo with newline' ' > > ++ test_must_fail git clone "$GIT_DAEMON_URL/repo$LF.git" dst > > 2>stderr && > > ++ test_i18ngrep newline.is.forbidden stderr > > ++' > > ++ > > + test_remote_error() > > + { > > + do_export=YesPlease > > +-- > > +2.17.1 > > + > > diff --git a/meta/recipes-devtools/git/git.inc > > b/meta/recipes-devtools/git/git.inc > > index 2b75bed055..a89dd42e8b 100644 > > --- a/meta/recipes-devtools/git/git.inc > > +++ b/meta/recipes-devtools/git/git.inc > > @@ -10,7 +10,9 @@ PROVIDES_append_class-native = " git-replacement-native" > > SRC_URI = > > "${KERNELORG_MIRROR}/software/scm/git/git-${PV}.tar.gz;name=tarball \ > > > > ${KERNELORG_MIRROR}/software/scm/git/git-manpages-${PV}.tar.gz;name=manpages > > \ > > file://CVE-2021-21300.patch \ > > - file://fixsort.patch" > > + file://fixsort.patch \ > > + file://CVE-2021-40330.patch \ > > + " > > > > S = "${WORKDIR}/git-${PV}" > > > > -- > > 2.30.1 (Apple Git-130) > > > > > > > > > > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#158865): https://lists.openembedded.org/g/openembedded-core/message/158865 Mute This Topic: https://lists.openembedded.org/mt/87300019/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
