diff mbox series

[kirkstone] go: fix CVE-2023-24537 Infinite loop in parsing

Message ID 20230420093128.1209660-1-vkumbhar@mvista.com
State Accepted, archived
Commit 15c07dff384ce4fb0e90f4f32c182a82101a1c82
Headers show
Series [kirkstone] go: fix CVE-2023-24537 Infinite loop in parsing | expand

Commit Message

Vivek Kumbhar April 20, 2023, 9:31 a.m. UTC
Setting a large line or column number using a //line directive can cause
integer overflow even in small source files.

Limit line and column numbers in //line directives to 2^30-1, which
is small enough to avoid int32 overflow on all reasonbly-sized files.

Fixes CVE-2023-24537
Fixes #59273
For #59180

Signed-off-by: Vivek Kumbhar <vkumbhar@mvista.com>
---
 meta/recipes-devtools/go/go-1.17.13.inc       |  1 +
 .../go/go-1.18/CVE-2023-24537.patch           | 72 +++++++++++++++++++
 2 files changed, 73 insertions(+)
 create mode 100644 meta/recipes-devtools/go/go-1.18/CVE-2023-24537.patch

Comments

Steve Sakoman April 20, 2023, 2:56 p.m. UTC | #1
Hi Vivek,

I'm getting fuzz errors with your patch:

WARNING: go-cross-core2-64-1.17.13-r0 do_patch: Fuzz detected:

Applying patch CVE-2023-24537.patch
patching file src/go/parser/parser_test.go
patching file src/go/scanner/scanner.go
Hunk #1 succeeded at 251 with fuzz 1.


The context lines in the patches can be updated with devtool:

    devtool modify go-cross-core2-64
    devtool finish --force-patch-refresh go-cross-core2-64 <layer_path>

Don't forget to review changes done by devtool!

WARNING: go-cross-core2-64-1.17.13-r0 do_patch: QA Issue: Patch log
indicates that patches do not apply cleanly. [patch-fuzz]

Steve

On Wed, Apr 19, 2023 at 11:31 PM vkumbhar <vkumbhar@mvista.com> wrote:
>
> Setting a large line or column number using a //line directive can cause
> integer overflow even in small source files.
>
> Limit line and column numbers in //line directives to 2^30-1, which
> is small enough to avoid int32 overflow on all reasonbly-sized files.
>
> Fixes CVE-2023-24537
> Fixes #59273
> For #59180
>
> Signed-off-by: Vivek Kumbhar <vkumbhar@mvista.com>
> ---
>  meta/recipes-devtools/go/go-1.17.13.inc       |  1 +
>  .../go/go-1.18/CVE-2023-24537.patch           | 72 +++++++++++++++++++
>  2 files changed, 73 insertions(+)
>  create mode 100644 meta/recipes-devtools/go/go-1.18/CVE-2023-24537.patch
>
> diff --git a/meta/recipes-devtools/go/go-1.17.13.inc b/meta/recipes-devtools/go/go-1.17.13.inc
> index 34d58aec2f..cda9227042 100644
> --- a/meta/recipes-devtools/go/go-1.17.13.inc
> +++ b/meta/recipes-devtools/go/go-1.17.13.inc
> @@ -27,6 +27,7 @@ SRC_URI += "\
>      file://add_godebug.patch \
>      file://cve-2022-41725.patch \
>      file://CVE-2022-41722.patch \
> +    file://CVE-2023-24537.patch \
>  "
>  SRC_URI[main.sha256sum] = "a1a48b23afb206f95e7bbaa9b898d965f90826f6f1d1fc0c1d784ada0cd300fd"
>
> diff --git a/meta/recipes-devtools/go/go-1.18/CVE-2023-24537.patch b/meta/recipes-devtools/go/go-1.18/CVE-2023-24537.patch
> new file mode 100644
> index 0000000000..7733a605e7
> --- /dev/null
> +++ b/meta/recipes-devtools/go/go-1.18/CVE-2023-24537.patch
> @@ -0,0 +1,72 @@
> +From bf8c7c575c8a552d9d79deb29e80854dc88528d0 Mon Sep 17 00:00:00 2001
> +From: Damien Neil <dneil@google.com>
> +Date: Mon, 20 Mar 2023 10:43:19 -0700
> +Subject: [PATCH] [release-branch.go1.20] mime/multipart: limit parsed mime
> + message sizes
> +
> +Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802456
> +Reviewed-by: Julie Qiu <julieqiu@google.com>
> +Reviewed-by: Roland Shoemaker <bracewell@google.com>
> +Run-TryBot: Damien Neil <dneil@google.com>
> +Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802611
> +Reviewed-by: Damien Neil <dneil@google.com>
> +Change-Id: Ifdfa192d54f722d781a4d8c5f35b5fb72d122168
> +Reviewed-on: https://go-review.googlesource.com/c/go/+/481986
> +Reviewed-by: Matthew Dempsky <mdempsky@google.com>
> +TryBot-Result: Gopher Robot <gobot@golang.org>
> +Run-TryBot: Michael Knyszek <mknyszek@google.com>
> +Auto-Submit: Michael Knyszek <mknyszek@google.com>
> +Signed-off-by: Vivek Kumbhar <vkumbhar@mvista.com>
> +Upstream-Status: Backport [https://github.com/golang/go/commit/126a1d02da82f93ede7ce0bd8d3c51ef627f2104]
> +CVE: CVE-2023-24537
> +Signed-off-by: Vivek Kumbhar <vkumbhar@mvista.com>
> +---
> + src/go/parser/parser_test.go | 16 ++++++++++++++++
> + src/go/scanner/scanner.go    |  7 +++++--
> + 2 files changed, 21 insertions(+), 2 deletions(-)
> +
> +diff --git a/src/go/parser/parser_test.go b/src/go/parser/parser_test.go
> +index 1a46c87..993df63 100644
> +--- a/src/go/parser/parser_test.go
> ++++ b/src/go/parser/parser_test.go
> +@@ -746,3 +746,19 @@ func TestScopeDepthLimit(t *testing.T) {
> +               }
> +       }
> + }
> ++
> ++// TestIssue59180 tests that line number overflow doesn't cause an infinite loop.
> ++func TestIssue59180(t *testing.T) {
> ++      testcases := []string{
> ++              "package p\n//line :9223372036854775806\n\n//",
> ++              "package p\n//line :1:9223372036854775806\n\n//",
> ++              "package p\n//line file:9223372036854775806\n\n//",
> ++      }
> ++
> ++      for _, src := range testcases {
> ++              _, err := ParseFile(token.NewFileSet(), "", src, ParseComments)
> ++              if err == nil {
> ++                      t.Errorf("ParseFile(%s) succeeded unexpectedly", src)
> ++              }
> ++      }
> ++}
> +diff --git a/src/go/scanner/scanner.go b/src/go/scanner/scanner.go
> +index f08e28c..2276e07 100644
> +--- a/src/go/scanner/scanner.go
> ++++ b/src/go/scanner/scanner.go
> +@@ -251,13 +251,16 @@ func (s *Scanner) updateLineInfo(next, offs int, text []byte) {
> +               return
> +       }
> +
> ++      // Put a cap on the maximum size of line and column numbers.
> ++      // 30 bits allows for some additional space before wrapping an int32.
> ++      const maxLineCol = 1<<30 - 1
> +       var line, col int
> +       i2, n2, ok2 := trailingDigits(text[:i-1])
> +       if ok2 {
> +               //line filename:line:col
> +               i, i2 = i2, i
> +               line, col = n2, n
> +-              if col == 0 {
> ++              if col == 0 || col > maxLineCol {
> +                       s.error(offs+i2, "invalid column number: "+string(text[i2:]))
> +                       return
> --
> 2.25.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#180243): https://lists.openembedded.org/g/openembedded-core/message/180243
> Mute This Topic: https://lists.openembedded.org/mt/98385268/3617601
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [sakoman@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Vivek Kumbhar April 21, 2023, 2:51 a.m. UTC | #2
v2 patch has been sent with fuzz error fix.

Kind regards,
Vivek

On Thu, Apr 20, 2023 at 8:27 PM Steve Sakoman <steve@sakoman.com> wrote:

> Hi Vivek,
>
> I'm getting fuzz errors with your patch:
>
> WARNING: go-cross-core2-64-1.17.13-r0 do_patch: Fuzz detected:
>
> Applying patch CVE-2023-24537.patch
> patching file src/go/parser/parser_test.go
> patching file src/go/scanner/scanner.go
> Hunk #1 succeeded at 251 with fuzz 1.
>
>
> The context lines in the patches can be updated with devtool:
>
>     devtool modify go-cross-core2-64
>     devtool finish --force-patch-refresh go-cross-core2-64 <layer_path>
>
> Don't forget to review changes done by devtool!
>
> WARNING: go-cross-core2-64-1.17.13-r0 do_patch: QA Issue: Patch log
> indicates that patches do not apply cleanly. [patch-fuzz]
>
> Steve
>
> On Wed, Apr 19, 2023 at 11:31 PM vkumbhar <vkumbhar@mvista.com> wrote:
> >
> > Setting a large line or column number using a //line directive can cause
> > integer overflow even in small source files.
> >
> > Limit line and column numbers in //line directives to 2^30-1, which
> > is small enough to avoid int32 overflow on all reasonbly-sized files.
> >
> > Fixes CVE-2023-24537
> > Fixes #59273
> > For #59180
> >
> > Signed-off-by: Vivek Kumbhar <vkumbhar@mvista.com>
> > ---
> >  meta/recipes-devtools/go/go-1.17.13.inc       |  1 +
> >  .../go/go-1.18/CVE-2023-24537.patch           | 72 +++++++++++++++++++
> >  2 files changed, 73 insertions(+)
> >  create mode 100644 meta/recipes-devtools/go/go-1.18/CVE-2023-24537.patch
> >
> > diff --git a/meta/recipes-devtools/go/go-1.17.13.inc
> b/meta/recipes-devtools/go/go-1.17.13.inc
> > index 34d58aec2f..cda9227042 100644
> > --- a/meta/recipes-devtools/go/go-1.17.13.inc
> > +++ b/meta/recipes-devtools/go/go-1.17.13.inc
> > @@ -27,6 +27,7 @@ SRC_URI += "\
> >      file://add_godebug.patch \
> >      file://cve-2022-41725.patch \
> >      file://CVE-2022-41722.patch \
> > +    file://CVE-2023-24537.patch \
> >  "
> >  SRC_URI[main.sha256sum] =
> "a1a48b23afb206f95e7bbaa9b898d965f90826f6f1d1fc0c1d784ada0cd300fd"
> >
> > diff --git a/meta/recipes-devtools/go/go-1.18/CVE-2023-24537.patch
> b/meta/recipes-devtools/go/go-1.18/CVE-2023-24537.patch
> > new file mode 100644
> > index 0000000000..7733a605e7
> > --- /dev/null
> > +++ b/meta/recipes-devtools/go/go-1.18/CVE-2023-24537.patch
> > @@ -0,0 +1,72 @@
> > +From bf8c7c575c8a552d9d79deb29e80854dc88528d0 Mon Sep 17 00:00:00 2001
> > +From: Damien Neil <dneil@google.com>
> > +Date: Mon, 20 Mar 2023 10:43:19 -0700
> > +Subject: [PATCH] [release-branch.go1.20] mime/multipart: limit parsed
> mime
> > + message sizes
> > +
> > +Reviewed-on:
> https://team-review.git.corp.google.com/c/golang/go-private/+/1802456
> > +Reviewed-by: Julie Qiu <julieqiu@google.com>
> > +Reviewed-by: Roland Shoemaker <bracewell@google.com>
> > +Run-TryBot: Damien Neil <dneil@google.com>
> > +Reviewed-on:
> https://team-review.git.corp.google.com/c/golang/go-private/+/1802611
> > +Reviewed-by: Damien Neil <dneil@google.com>
> > +Change-Id: Ifdfa192d54f722d781a4d8c5f35b5fb72d122168
> > +Reviewed-on: https://go-review.googlesource.com/c/go/+/481986
> > +Reviewed-by: Matthew Dempsky <mdempsky@google.com>
> > +TryBot-Result: Gopher Robot <gobot@golang.org>
> > +Run-TryBot: Michael Knyszek <mknyszek@google.com>
> > +Auto-Submit: Michael Knyszek <mknyszek@google.com>
> > +Signed-off-by: Vivek Kumbhar <vkumbhar@mvista.com>
> > +Upstream-Status: Backport [
> https://github.com/golang/go/commit/126a1d02da82f93ede7ce0bd8d3c51ef627f2104
> ]
> > +CVE: CVE-2023-24537
> > +Signed-off-by: Vivek Kumbhar <vkumbhar@mvista.com>
> > +---
> > + src/go/parser/parser_test.go | 16 ++++++++++++++++
> > + src/go/scanner/scanner.go    |  7 +++++--
> > + 2 files changed, 21 insertions(+), 2 deletions(-)
> > +
> > +diff --git a/src/go/parser/parser_test.go b/src/go/parser/parser_test.go
> > +index 1a46c87..993df63 100644
> > +--- a/src/go/parser/parser_test.go
> > ++++ b/src/go/parser/parser_test.go
> > +@@ -746,3 +746,19 @@ func TestScopeDepthLimit(t *testing.T) {
> > +               }
> > +       }
> > + }
> > ++
> > ++// TestIssue59180 tests that line number overflow doesn't cause an
> infinite loop.
> > ++func TestIssue59180(t *testing.T) {
> > ++      testcases := []string{
> > ++              "package p\n//line :9223372036854775806\n\n//",
> > ++              "package p\n//line :1:9223372036854775806\n\n//",
> > ++              "package p\n//line file:9223372036854775806\n\n//",
> > ++      }
> > ++
> > ++      for _, src := range testcases {
> > ++              _, err := ParseFile(token.NewFileSet(), "", src,
> ParseComments)
> > ++              if err == nil {
> > ++                      t.Errorf("ParseFile(%s) succeeded unexpectedly",
> src)
> > ++              }
> > ++      }
> > ++}
> > +diff --git a/src/go/scanner/scanner.go b/src/go/scanner/scanner.go
> > +index f08e28c..2276e07 100644
> > +--- a/src/go/scanner/scanner.go
> > ++++ b/src/go/scanner/scanner.go
> > +@@ -251,13 +251,16 @@ func (s *Scanner) updateLineInfo(next, offs int,
> text []byte) {
> > +               return
> > +       }
> > +
> > ++      // Put a cap on the maximum size of line and column numbers.
> > ++      // 30 bits allows for some additional space before wrapping an
> int32.
> > ++      const maxLineCol = 1<<30 - 1
> > +       var line, col int
> > +       i2, n2, ok2 := trailingDigits(text[:i-1])
> > +       if ok2 {
> > +               //line filename:line:col
> > +               i, i2 = i2, i
> > +               line, col = n2, n
> > +-              if col == 0 {
> > ++              if col == 0 || col > maxLineCol {
> > +                       s.error(offs+i2, "invalid column number:
> "+string(text[i2:]))
> > +                       return
> > --
> > 2.25.1
> >
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#180243):
> https://lists.openembedded.org/g/openembedded-core/message/180243
> > Mute This Topic: https://lists.openembedded.org/mt/98385268/3617601
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> sakoman@gmail.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 34d58aec2f..cda9227042 100644
--- a/meta/recipes-devtools/go/go-1.17.13.inc
+++ b/meta/recipes-devtools/go/go-1.17.13.inc
@@ -27,6 +27,7 @@  SRC_URI += "\
     file://add_godebug.patch \
     file://cve-2022-41725.patch \
     file://CVE-2022-41722.patch \
+    file://CVE-2023-24537.patch \
 "
 SRC_URI[main.sha256sum] = "a1a48b23afb206f95e7bbaa9b898d965f90826f6f1d1fc0c1d784ada0cd300fd"
 
diff --git a/meta/recipes-devtools/go/go-1.18/CVE-2023-24537.patch b/meta/recipes-devtools/go/go-1.18/CVE-2023-24537.patch
new file mode 100644
index 0000000000..7733a605e7
--- /dev/null
+++ b/meta/recipes-devtools/go/go-1.18/CVE-2023-24537.patch
@@ -0,0 +1,72 @@ 
+From bf8c7c575c8a552d9d79deb29e80854dc88528d0 Mon Sep 17 00:00:00 2001
+From: Damien Neil <dneil@google.com>
+Date: Mon, 20 Mar 2023 10:43:19 -0700
+Subject: [PATCH] [release-branch.go1.20] mime/multipart: limit parsed mime
+ message sizes
+
+Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802456
+Reviewed-by: Julie Qiu <julieqiu@google.com>
+Reviewed-by: Roland Shoemaker <bracewell@google.com>
+Run-TryBot: Damien Neil <dneil@google.com>
+Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802611
+Reviewed-by: Damien Neil <dneil@google.com>
+Change-Id: Ifdfa192d54f722d781a4d8c5f35b5fb72d122168
+Reviewed-on: https://go-review.googlesource.com/c/go/+/481986
+Reviewed-by: Matthew Dempsky <mdempsky@google.com>
+TryBot-Result: Gopher Robot <gobot@golang.org>
+Run-TryBot: Michael Knyszek <mknyszek@google.com>
+Auto-Submit: Michael Knyszek <mknyszek@google.com>
+Signed-off-by: Vivek Kumbhar <vkumbhar@mvista.com>
+Upstream-Status: Backport [https://github.com/golang/go/commit/126a1d02da82f93ede7ce0bd8d3c51ef627f2104]
+CVE: CVE-2023-24537
+Signed-off-by: Vivek Kumbhar <vkumbhar@mvista.com>
+---
+ src/go/parser/parser_test.go | 16 ++++++++++++++++
+ src/go/scanner/scanner.go    |  7 +++++--
+ 2 files changed, 21 insertions(+), 2 deletions(-)
+
+diff --git a/src/go/parser/parser_test.go b/src/go/parser/parser_test.go
+index 1a46c87..993df63 100644
+--- a/src/go/parser/parser_test.go
++++ b/src/go/parser/parser_test.go
+@@ -746,3 +746,19 @@ func TestScopeDepthLimit(t *testing.T) {
+		}
+	}
+ }
++
++// TestIssue59180 tests that line number overflow doesn't cause an infinite loop.
++func TestIssue59180(t *testing.T) {
++	testcases := []string{
++		"package p\n//line :9223372036854775806\n\n//",
++		"package p\n//line :1:9223372036854775806\n\n//",
++		"package p\n//line file:9223372036854775806\n\n//",
++	}
++
++	for _, src := range testcases {
++		_, err := ParseFile(token.NewFileSet(), "", src, ParseComments)
++		if err == nil {
++			t.Errorf("ParseFile(%s) succeeded unexpectedly", src)
++		}
++	}
++}
+diff --git a/src/go/scanner/scanner.go b/src/go/scanner/scanner.go
+index f08e28c..2276e07 100644
+--- a/src/go/scanner/scanner.go
++++ b/src/go/scanner/scanner.go
+@@ -251,13 +251,16 @@ func (s *Scanner) updateLineInfo(next, offs int, text []byte) {
+		return
+	}
+
++	// Put a cap on the maximum size of line and column numbers.
++	// 30 bits allows for some additional space before wrapping an int32.
++	const maxLineCol = 1<<30 - 1
+	var line, col int
+	i2, n2, ok2 := trailingDigits(text[:i-1])
+	if ok2 {
+		//line filename:line:col
+		i, i2 = i2, i
+		line, col = n2, n
+-		if col == 0 {
++		if col == 0 || col > maxLineCol {
+			s.error(offs+i2, "invalid column number: "+string(text[i2:]))
+			return