diff mbox series

[kirkstone] go-runtime: Security fix for CVE-2022-41722

Message ID 20230418115423.30752-1-skulkarni@mvista.com
State New, archived
Headers show
Series [kirkstone] go-runtime: Security fix for CVE-2022-41722 | expand

Commit Message

Shubham Kulkarni April 18, 2023, 11:54 a.m. UTC
From: Shubham Kulkarni <skulkarni@mvista.com>

path/filepath: do not Clean("a/../c:/b") into c:\b on Windows

Backport from https://github.com/golang/go/commit/bdf07c2e168baf736e4c057279ca12a4d674f18c

Signed-off-by: Shubham Kulkarni <skulkarni@mvista.com>
---
 meta/recipes-devtools/go/go-1.17.13.inc       |   1 +
 .../go/go-1.18/CVE-2022-41722.patch           | 102 ++++++++++++++++++
 2 files changed, 103 insertions(+)
 create mode 100644 meta/recipes-devtools/go/go-1.18/CVE-2022-41722.patch

Comments

Steve Sakoman April 18, 2023, 2:34 p.m. UTC | #1
There were a couple of issues with this patch.  I've fixed both so no
need to resubmit, but in the future please be sure to check the
following:

1. Patch should be based on the latest kirkstone head -- you were
using an earlier state which was missing "go: fix CVE-2022-41724,
41725" so the patch didn't apply.
2. CVE patch files should have a CVE: tag in addition to the
Upstream-status: and Signed-off-by: tags

Thanks for helping fix CVEs!

Steve

On Tue, Apr 18, 2023 at 1:54 AM Shubham Kulkarni <skulkarni@mvista.com> wrote:
>
> From: Shubham Kulkarni <skulkarni@mvista.com>
>
> path/filepath: do not Clean("a/../c:/b") into c:\b on Windows
>
> Backport from https://github.com/golang/go/commit/bdf07c2e168baf736e4c057279ca12a4d674f18c
>
> Signed-off-by: Shubham Kulkarni <skulkarni@mvista.com>
> ---
>  meta/recipes-devtools/go/go-1.17.13.inc       |   1 +
>  .../go/go-1.18/CVE-2022-41722.patch           | 102 ++++++++++++++++++
>  2 files changed, 103 insertions(+)
>  create mode 100644 meta/recipes-devtools/go/go-1.18/CVE-2022-41722.patch
>
> diff --git a/meta/recipes-devtools/go/go-1.17.13.inc b/meta/recipes-devtools/go/go-1.17.13.inc
> index 14d58932dc..d104e34408 100644
> --- a/meta/recipes-devtools/go/go-1.17.13.inc
> +++ b/meta/recipes-devtools/go/go-1.17.13.inc
> @@ -23,6 +23,7 @@ SRC_URI += "\
>      file://CVE-2022-2879.patch \
>      file://CVE-2022-41720.patch \
>      file://CVE-2022-41723.patch \
> +    file://CVE-2022-41722.patch \
>  "
>  SRC_URI[main.sha256sum] = "a1a48b23afb206f95e7bbaa9b898d965f90826f6f1d1fc0c1d784ada0cd300fd"
>
> diff --git a/meta/recipes-devtools/go/go-1.18/CVE-2022-41722.patch b/meta/recipes-devtools/go/go-1.18/CVE-2022-41722.patch
> new file mode 100644
> index 0000000000..447c3d45bd
> --- /dev/null
> +++ b/meta/recipes-devtools/go/go-1.18/CVE-2022-41722.patch
> @@ -0,0 +1,102 @@
> +From a826b19625caebed6dd0f3fbd9d0111f6c83737c Mon Sep 17 00:00:00 2001
> +From: Damien Neil <dneil@google.com>
> +Date: Mon, 12 Dec 2022 16:43:37 -0800
> +Subject: [PATCH] path/filepath: do not Clean("a/../c:/b") into c:\b on Windows
> +
> +Do not permit Clean to convert a relative path into one starting
> +with a drive reference. This change causes Clean to insert a .
> +path element at the start of a path when the original path does not
> +start with a volume name, and the first path element would contain
> +a colon.
> +
> +This may introduce a spurious but harmless . path element under
> +some circumstances. For example, Clean("a/../b:/../c") becomes `.\c`.
> +
> +This reverts CL 401595, since the change here supersedes the one
> +in that CL.
> +
> +Thanks to RyotaK (https://twitter.com/ryotkak) for reporting this issue.
> +
> +Updates #57274
> +Fixes #57276
> +Fixes CVE-2022-41722
> +
> +Change-Id: I837446285a03aa74c79d7642720e01f354c2ca17
> +Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1675249
> +Reviewed-by: Roland Shoemaker <bracewell@google.com>
> +Run-TryBot: Damien Neil <dneil@google.com>
> +Reviewed-by: Julie Qiu <julieqiu@google.com>
> +TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
> +(cherry picked from commit 8ca37f4813ef2f64600c92b83f17c9f3ca6c03a5)
> +Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1728944
> +Run-TryBot: Roland Shoemaker <bracewell@google.com>
> +Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
> +Reviewed-by: Damien Neil <dneil@google.com>
> +Reviewed-on: https://go-review.googlesource.com/c/go/+/468119
> +Reviewed-by: Than McIntosh <thanm@google.com>
> +Run-TryBot: Michael Pratt <mpratt@google.com>
> +TryBot-Result: Gopher Robot <gobot@golang.org>
> +Auto-Submit: Michael Pratt <mpratt@google.com>
> +
> +Upstream-Status: Backport from https://github.com/golang/go/commit/bdf07c2e168baf736e4c057279ca12a4d674f18
> +Signed-off-by: Shubham Kulkarni <skulkarni@mvista.com>
> +---
> + src/path/filepath/path.go | 27 ++++++++++++++-------------
> + 1 file changed, 14 insertions(+), 13 deletions(-)
> +
> +diff --git a/src/path/filepath/path.go b/src/path/filepath/path.go
> +index 8300a32..94621a0 100644
> +--- a/src/path/filepath/path.go
> ++++ b/src/path/filepath/path.go
> +@@ -15,6 +15,7 @@ import (
> +       "errors"
> +       "io/fs"
> +       "os"
> ++      "runtime"
> +       "sort"
> +       "strings"
> + )
> +@@ -117,21 +118,9 @@ func Clean(path string) string {
> +               case os.IsPathSeparator(path[r]):
> +                       // empty path element
> +                       r++
> +-              case path[r] == '.' && r+1 == n:
> ++              case path[r] == '.' && (r+1 == n || os.IsPathSeparator(path[r+1])):
> +                       // . element
> +                       r++
> +-              case path[r] == '.' && os.IsPathSeparator(path[r+1]):
> +-                      // ./ element
> +-                      r++
> +-
> +-                      for r < len(path) && os.IsPathSeparator(path[r]) {
> +-                              r++
> +-                      }
> +-                      if out.w == 0 && volumeNameLen(path[r:]) > 0 {
> +-                              // When joining prefix "." and an absolute path on Windows,
> +-                              // the prefix should not be removed.
> +-                              out.append('.')
> +-                      }
> +               case path[r] == '.' && path[r+1] == '.' && (r+2 == n || os.IsPathSeparator(path[r+2])):
> +                       // .. element: remove to last separator
> +                       r += 2
> +@@ -157,6 +146,18 @@ func Clean(path string) string {
> +                       if rooted && out.w != 1 || !rooted && out.w != 0 {
> +                               out.append(Separator)
> +                       }
> ++                      // If a ':' appears in the path element at the start of a Windows path,
> ++                      // insert a .\ at the beginning to avoid converting relative paths
> ++                      // like a/../c: into c:.
> ++                      if runtime.GOOS == "windows" && out.w == 0 && out.volLen == 0 && r != 0 {
> ++                              for i := r; i < n && !os.IsPathSeparator(path[i]); i++ {
> ++                                      if path[i] == ':' {
> ++                                              out.append('.')
> ++                                              out.append(Separator)
> ++                                              break
> ++                                      }
> ++                              }
> ++                      }
> +                       // copy element
> +                       for ; r < n && !os.IsPathSeparator(path[r]); r++ {
> +                               out.append(path[r])
> +--
> +2.7.4
> --
> 2.33.0
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#180187): https://lists.openembedded.org/g/openembedded-core/message/180187
> Mute This Topic: https://lists.openembedded.org/mt/98341957/3620601
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [steve@sakoman.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Shubham Kulkarni April 19, 2023, 5:55 a.m. UTC | #2
Thank you Steve! Apologies for the inconvenience caused. I will take care
next time.

Thanks,
Shubham

On Tue, Apr 18, 2023 at 8:04 PM Steve Sakoman <steve@sakoman.com> wrote:

> There were a couple of issues with this patch.  I've fixed both so no
> need to resubmit, but in the future please be sure to check the
> following:
>
> 1. Patch should be based on the latest kirkstone head -- you were
> using an earlier state which was missing "go: fix CVE-2022-41724,
> 41725" so the patch didn't apply.
> 2. CVE patch files should have a CVE: tag in addition to the
> Upstream-status: and Signed-off-by: tags
>
> Thanks for helping fix CVEs!
>
> Steve
>
> On Tue, Apr 18, 2023 at 1:54 AM Shubham Kulkarni <skulkarni@mvista.com>
> wrote:
> >
> > From: Shubham Kulkarni <skulkarni@mvista.com>
> >
> > path/filepath: do not Clean("a/../c:/b") into c:\b on Windows
> >
> > Backport from
> https://github.com/golang/go/commit/bdf07c2e168baf736e4c057279ca12a4d674f18c
> >
> > Signed-off-by: Shubham Kulkarni <skulkarni@mvista.com>
> > ---
> >  meta/recipes-devtools/go/go-1.17.13.inc       |   1 +
> >  .../go/go-1.18/CVE-2022-41722.patch           | 102 ++++++++++++++++++
> >  2 files changed, 103 insertions(+)
> >  create mode 100644 meta/recipes-devtools/go/go-1.18/CVE-2022-41722.patch
> >
> > diff --git a/meta/recipes-devtools/go/go-1.17.13.inc
> b/meta/recipes-devtools/go/go-1.17.13.inc
> > index 14d58932dc..d104e34408 100644
> > --- a/meta/recipes-devtools/go/go-1.17.13.inc
> > +++ b/meta/recipes-devtools/go/go-1.17.13.inc
> > @@ -23,6 +23,7 @@ SRC_URI += "\
> >      file://CVE-2022-2879.patch \
> >      file://CVE-2022-41720.patch \
> >      file://CVE-2022-41723.patch \
> > +    file://CVE-2022-41722.patch \
> >  "
> >  SRC_URI[main.sha256sum] =
> "a1a48b23afb206f95e7bbaa9b898d965f90826f6f1d1fc0c1d784ada0cd300fd"
> >
> > diff --git a/meta/recipes-devtools/go/go-1.18/CVE-2022-41722.patch
> b/meta/recipes-devtools/go/go-1.18/CVE-2022-41722.patch
> > new file mode 100644
> > index 0000000000..447c3d45bd
> > --- /dev/null
> > +++ b/meta/recipes-devtools/go/go-1.18/CVE-2022-41722.patch
> > @@ -0,0 +1,102 @@
> > +From a826b19625caebed6dd0f3fbd9d0111f6c83737c Mon Sep 17 00:00:00 2001
> > +From: Damien Neil <dneil@google.com>
> > +Date: Mon, 12 Dec 2022 16:43:37 -0800
> > +Subject: [PATCH] path/filepath: do not Clean("a/../c:/b") into c:\b on
> Windows
> > +
> > +Do not permit Clean to convert a relative path into one starting
> > +with a drive reference. This change causes Clean to insert a .
> > +path element at the start of a path when the original path does not
> > +start with a volume name, and the first path element would contain
> > +a colon.
> > +
> > +This may introduce a spurious but harmless . path element under
> > +some circumstances. For example, Clean("a/../b:/../c") becomes `.\c`.
> > +
> > +This reverts CL 401595, since the change here supersedes the one
> > +in that CL.
> > +
> > +Thanks to RyotaK (https://twitter.com/ryotkak) for reporting this
> issue.
> > +
> > +Updates #57274
> > +Fixes #57276
> > +Fixes CVE-2022-41722
> > +
> > +Change-Id: I837446285a03aa74c79d7642720e01f354c2ca17
> > +Reviewed-on:
> https://team-review.git.corp.google.com/c/golang/go-private/+/1675249
> > +Reviewed-by: Roland Shoemaker <bracewell@google.com>
> > +Run-TryBot: Damien Neil <dneil@google.com>
> > +Reviewed-by: Julie Qiu <julieqiu@google.com>
> > +TryBot-Result: Security TryBots <
> security-trybots@go-security-trybots.iam.gserviceaccount.com>
> > +(cherry picked from commit 8ca37f4813ef2f64600c92b83f17c9f3ca6c03a5)
> > +Reviewed-on:
> https://team-review.git.corp.google.com/c/golang/go-private/+/1728944
> > +Run-TryBot: Roland Shoemaker <bracewell@google.com>
> > +Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
> > +Reviewed-by: Damien Neil <dneil@google.com>
> > +Reviewed-on: https://go-review.googlesource.com/c/go/+/468119
> > +Reviewed-by: Than McIntosh <thanm@google.com>
> > +Run-TryBot: Michael Pratt <mpratt@google.com>
> > +TryBot-Result: Gopher Robot <gobot@golang.org>
> > +Auto-Submit: Michael Pratt <mpratt@google.com>
> > +
> > +Upstream-Status: Backport from
> https://github.com/golang/go/commit/bdf07c2e168baf736e4c057279ca12a4d674f18
> > +Signed-off-by: Shubham Kulkarni <skulkarni@mvista.com>
> > +---
> > + src/path/filepath/path.go | 27 ++++++++++++++-------------
> > + 1 file changed, 14 insertions(+), 13 deletions(-)
> > +
> > +diff --git a/src/path/filepath/path.go b/src/path/filepath/path.go
> > +index 8300a32..94621a0 100644
> > +--- a/src/path/filepath/path.go
> > ++++ b/src/path/filepath/path.go
> > +@@ -15,6 +15,7 @@ import (
> > +       "errors"
> > +       "io/fs"
> > +       "os"
> > ++      "runtime"
> > +       "sort"
> > +       "strings"
> > + )
> > +@@ -117,21 +118,9 @@ func Clean(path string) string {
> > +               case os.IsPathSeparator(path[r]):
> > +                       // empty path element
> > +                       r++
> > +-              case path[r] == '.' && r+1 == n:
> > ++              case path[r] == '.' && (r+1 == n ||
> os.IsPathSeparator(path[r+1])):
> > +                       // . element
> > +                       r++
> > +-              case path[r] == '.' && os.IsPathSeparator(path[r+1]):
> > +-                      // ./ element
> > +-                      r++
> > +-
> > +-                      for r < len(path) && os.IsPathSeparator(path[r])
> {
> > +-                              r++
> > +-                      }
> > +-                      if out.w == 0 && volumeNameLen(path[r:]) > 0 {
> > +-                              // When joining prefix "." and an
> absolute path on Windows,
> > +-                              // the prefix should not be removed.
> > +-                              out.append('.')
> > +-                      }
> > +               case path[r] == '.' && path[r+1] == '.' && (r+2 == n ||
> os.IsPathSeparator(path[r+2])):
> > +                       // .. element: remove to last separator
> > +                       r += 2
> > +@@ -157,6 +146,18 @@ func Clean(path string) string {
> > +                       if rooted && out.w != 1 || !rooted && out.w != 0
> {
> > +                               out.append(Separator)
> > +                       }
> > ++                      // If a ':' appears in the path element at the
> start of a Windows path,
> > ++                      // insert a .\ at the beginning to avoid
> converting relative paths
> > ++                      // like a/../c: into c:.
> > ++                      if runtime.GOOS == "windows" && out.w == 0 &&
> out.volLen == 0 && r != 0 {
> > ++                              for i := r; i < n &&
> !os.IsPathSeparator(path[i]); i++ {
> > ++                                      if path[i] == ':' {
> > ++                                              out.append('.')
> > ++                                              out.append(Separator)
> > ++                                              break
> > ++                                      }
> > ++                              }
> > ++                      }
> > +                       // copy element
> > +                       for ; r < n && !os.IsPathSeparator(path[r]); r++
> {
> > +                               out.append(path[r])
> > +--
> > +2.7.4
> > --
> > 2.33.0
> >
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#180187):
> https://lists.openembedded.org/g/openembedded-core/message/180187
> > Mute This Topic: https://lists.openembedded.org/mt/98341957/3620601
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> steve@sakoman.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
>
diff mbox series

Patch

diff --git a/meta/recipes-devtools/go/go-1.17.13.inc b/meta/recipes-devtools/go/go-1.17.13.inc
index 14d58932dc..d104e34408 100644
--- a/meta/recipes-devtools/go/go-1.17.13.inc
+++ b/meta/recipes-devtools/go/go-1.17.13.inc
@@ -23,6 +23,7 @@  SRC_URI += "\
     file://CVE-2022-2879.patch \
     file://CVE-2022-41720.patch \
     file://CVE-2022-41723.patch \
+    file://CVE-2022-41722.patch \
 "
 SRC_URI[main.sha256sum] = "a1a48b23afb206f95e7bbaa9b898d965f90826f6f1d1fc0c1d784ada0cd300fd"
 
diff --git a/meta/recipes-devtools/go/go-1.18/CVE-2022-41722.patch b/meta/recipes-devtools/go/go-1.18/CVE-2022-41722.patch
new file mode 100644
index 0000000000..447c3d45bd
--- /dev/null
+++ b/meta/recipes-devtools/go/go-1.18/CVE-2022-41722.patch
@@ -0,0 +1,102 @@ 
+From a826b19625caebed6dd0f3fbd9d0111f6c83737c Mon Sep 17 00:00:00 2001
+From: Damien Neil <dneil@google.com>
+Date: Mon, 12 Dec 2022 16:43:37 -0800
+Subject: [PATCH] path/filepath: do not Clean("a/../c:/b") into c:\b on Windows
+
+Do not permit Clean to convert a relative path into one starting
+with a drive reference. This change causes Clean to insert a .
+path element at the start of a path when the original path does not
+start with a volume name, and the first path element would contain
+a colon.
+
+This may introduce a spurious but harmless . path element under
+some circumstances. For example, Clean("a/../b:/../c") becomes `.\c`.
+
+This reverts CL 401595, since the change here supersedes the one
+in that CL.
+
+Thanks to RyotaK (https://twitter.com/ryotkak) for reporting this issue.
+
+Updates #57274
+Fixes #57276
+Fixes CVE-2022-41722
+
+Change-Id: I837446285a03aa74c79d7642720e01f354c2ca17
+Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1675249
+Reviewed-by: Roland Shoemaker <bracewell@google.com>
+Run-TryBot: Damien Neil <dneil@google.com>
+Reviewed-by: Julie Qiu <julieqiu@google.com>
+TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
+(cherry picked from commit 8ca37f4813ef2f64600c92b83f17c9f3ca6c03a5)
+Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1728944
+Run-TryBot: Roland Shoemaker <bracewell@google.com>
+Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
+Reviewed-by: Damien Neil <dneil@google.com>
+Reviewed-on: https://go-review.googlesource.com/c/go/+/468119
+Reviewed-by: Than McIntosh <thanm@google.com>
+Run-TryBot: Michael Pratt <mpratt@google.com>
+TryBot-Result: Gopher Robot <gobot@golang.org>
+Auto-Submit: Michael Pratt <mpratt@google.com>
+
+Upstream-Status: Backport from https://github.com/golang/go/commit/bdf07c2e168baf736e4c057279ca12a4d674f18
+Signed-off-by: Shubham Kulkarni <skulkarni@mvista.com>
+---
+ src/path/filepath/path.go | 27 ++++++++++++++-------------
+ 1 file changed, 14 insertions(+), 13 deletions(-)
+
+diff --git a/src/path/filepath/path.go b/src/path/filepath/path.go
+index 8300a32..94621a0 100644
+--- a/src/path/filepath/path.go
++++ b/src/path/filepath/path.go
+@@ -15,6 +15,7 @@ import (
+	"errors"
+	"io/fs"
+	"os"
++	"runtime"
+	"sort"
+	"strings"
+ )
+@@ -117,21 +118,9 @@ func Clean(path string) string {
+		case os.IsPathSeparator(path[r]):
+			// empty path element
+			r++
+-		case path[r] == '.' && r+1 == n:
++		case path[r] == '.' && (r+1 == n || os.IsPathSeparator(path[r+1])):
+			// . element
+			r++
+-		case path[r] == '.' && os.IsPathSeparator(path[r+1]):
+-			// ./ element
+-			r++
+-
+-			for r < len(path) && os.IsPathSeparator(path[r]) {
+-				r++
+-			}
+-			if out.w == 0 && volumeNameLen(path[r:]) > 0 {
+-				// When joining prefix "." and an absolute path on Windows,
+-				// the prefix should not be removed.
+-				out.append('.')
+-			}
+		case path[r] == '.' && path[r+1] == '.' && (r+2 == n || os.IsPathSeparator(path[r+2])):
+			// .. element: remove to last separator
+			r += 2
+@@ -157,6 +146,18 @@ func Clean(path string) string {
+			if rooted && out.w != 1 || !rooted && out.w != 0 {
+				out.append(Separator)
+			}
++			// If a ':' appears in the path element at the start of a Windows path,
++			// insert a .\ at the beginning to avoid converting relative paths
++			// like a/../c: into c:.
++			if runtime.GOOS == "windows" && out.w == 0 && out.volLen == 0 && r != 0 {
++				for i := r; i < n && !os.IsPathSeparator(path[i]); i++ {
++					if path[i] == ':' {
++						out.append('.')
++						out.append(Separator)
++						break
++					}
++				}
++			}
+			// copy element
+			for ; r < n && !os.IsPathSeparator(path[r]); r++ {
+				out.append(path[r])
+--
+2.7.4