golang-1.17:fix CVE-2023-24534,CVE-2023-24536,CVE-2023-24537,CVE-2023-24538

CVE:CVE-2023-24534,CVE-2023-24536,CVE-2023-24537,CVE-2023-24538
Reference:https://go-review.googlesource.com/c/go/+/481982,
  https://go-review.googlesource.com/c/go/+/481986,
  https://go-review.googlesource.com/c/go/+/481987,
  https://go-review.googlesource.com/c/go/+/481983,
  https://go-review.googlesource.com/c/go/+/481984,
  https://go-review.googlesource.com/c/go/+/481985
Type:CVE
reason: fix CVE-2023-24534,CVE-2023-24536,CVE-2023-24537,CVE-2023-24538
This commit is contained in:
hanchao 2023-04-13 17:16:45 +08:00 committed by hanchao
parent dc342486df
commit eeac9110d3
8 changed files with 1437 additions and 34 deletions

View File

@ -1,8 +1,7 @@
From 45dc236afb19968c3628ae981d4b1b6590eb0a1c Mon Sep 17 00:00:00 2001
From ac28aeccf77fed9d34b658fbe3507d01713ae840 Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Wed, 25 Jan 2023 09:27:01 -0800
Subject: [PATCH] [release-branch.go1.19] mime/multipart: limit memory/inode
consumption of ReadForm
Subject: [PATCH] mime/multipart: limit memory/inode consumption of ReadForm
Reader.ReadForm is documented as storing "up to maxMemory bytes + 10MB"
in memory. Parsed forms can consume substantially more memory than
@ -57,30 +56,41 @@ Auto-Submit: Michael Pratt <mpratt@google.com>
Reference:https://go-review.googlesource.com/c/go/+/468116
Conflict:NA
---
src/mime/multipart/formdata.go | 157 ++++++++++++++++++++++-----
src/mime/multipart/formdata_test.go | 140 +++++++++++++++++++++++-
src/mime/multipart/multipart.go | 29 +++--
src/internal/godebug/godebug.go | 34 +++++++
src/internal/godebug/godebug_test.go | 34 +++++++
src/mime/multipart/formdata.go | 132 ++++++++++++++++++++-----
src/mime/multipart/formdata_test.go | 140 ++++++++++++++++++++++++++-
src/mime/multipart/multipart.go | 29 ++++--
src/mime/multipart/readmimeheader.go | 14 +++
src/net/http/request_test.go | 2 +-
src/net/textproto/reader.go | 18 +++
6 files changed, 321 insertions(+), 39 deletions(-)
src/net/textproto/reader.go | 18 ++++
8 files changed, 364 insertions(+), 39 deletions(-)
create mode 100644 src/internal/godebug/godebug.go
create mode 100644 src/internal/godebug/godebug_test.go
create mode 100644 src/mime/multipart/readmimeheader.go
diff --git a/src/mime/multipart/formdata.go b/src/mime/multipart/formdata.go
index fca5f9e15f..b0b4fb83a5 100644
--- a/src/mime/multipart/formdata.go
+++ b/src/mime/multipart/formdata.go
@@ -31,25 +31,86 @@ func (r *Reader) ReadForm(maxMemory int64) (*Form, error) {
return r.readForm(maxMemory)
}
+// godebugGet returns the value for the provided GODEBUG key.
+func godebugGet(key string) string {
+ return godebugGet2(os.Getenv("GODEBUG"), key)
diff --git a/src/internal/godebug/godebug.go b/src/internal/godebug/godebug.go
new file mode 100644
index 0000000..ac434e5
--- /dev/null
+++ b/src/internal/godebug/godebug.go
@@ -0,0 +1,34 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package godebug parses the GODEBUG environment variable.
+package godebug
+
+import "os"
+
+// Get returns the value for the provided GODEBUG key.
+func Get(key string) string {
+ return get(os.Getenv("GODEBUG"), key)
+}
+
+// godebugGet2 returns the value part of key=value in s (a GODEBUG value).
+func godebugGet2(s, key string) string {
+// get returns the value part of key=value in s (a GODEBUG value).
+func get(s, key string) string {
+ for i := 0; i < len(s)-len(key)-1; i++ {
+ if i > 0 && s[i-1] != ',' {
+ continue
@ -99,7 +109,60 @@ index fca5f9e15f..b0b4fb83a5 100644
+ }
+ return ""
+}
diff --git a/src/internal/godebug/godebug_test.go b/src/internal/godebug/godebug_test.go
new file mode 100644
index 0000000..41b9117
--- /dev/null
+++ b/src/internal/godebug/godebug_test.go
@@ -0,0 +1,34 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package godebug
+
+import "testing"
+
+func TestGet(t *testing.T) {
+ tests := []struct {
+ godebug string
+ key string
+ want string
+ }{
+ {"", "", ""},
+ {"", "foo", ""},
+ {"foo=bar", "foo", "bar"},
+ {"foo=bar,after=x", "foo", "bar"},
+ {"before=x,foo=bar,after=x", "foo", "bar"},
+ {"before=x,foo=bar", "foo", "bar"},
+ {",,,foo=bar,,,", "foo", "bar"},
+ {"foodecoy=wrong,foo=bar", "foo", "bar"},
+ {"foo=", "foo", ""},
+ {"foo", "foo", ""},
+ {",foo", "foo", ""},
+ {"foo=bar,baz", "loooooooong", ""},
+ }
+ for _, tt := range tests {
+ got := get(tt.godebug, tt.key)
+ if got != tt.want {
+ t.Errorf("get(%q, %q) = %q; want %q", tt.godebug, tt.key, got, tt.want)
+ }
+ }
+}
diff --git a/src/mime/multipart/formdata.go b/src/mime/multipart/formdata.go
index fca5f9e..a7d4ca9 100644
--- a/src/mime/multipart/formdata.go
+++ b/src/mime/multipart/formdata.go
@@ -7,6 +7,7 @@ package multipart
import (
"bytes"
"errors"
+ "internal/godebug"
"io"
"math"
"net/textproto"
@@ -33,23 +34,58 @@ func (r *Reader) ReadForm(maxMemory int64) (*Form, error) {
func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
form := &Form{make(map[string][]string), make(map[string][]*FileHeader)}
+ var (
@ -107,7 +170,7 @@ index fca5f9e15f..b0b4fb83a5 100644
+ fileOff int64
+ )
+ numDiskFiles := 0
+ multipartFiles := godebugGet("multipartfiles")
+ multipartFiles := godebug.Get("multipartfiles")
+ combineFiles := multipartFiles != "distinct"
defer func() {
+ if file != nil {
@ -163,7 +226,7 @@ index fca5f9e15f..b0b4fb83a5 100644
if err == io.EOF {
break
}
@@ -63,16 +124,27 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
@@ -63,16 +99,27 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
}
filename := p.FileName()
@ -194,7 +257,7 @@ index fca5f9e15f..b0b4fb83a5 100644
return nil, ErrMessageTooLarge
}
form.Value[name] = append(form.Value[name], b.String())
@@ -80,35 +152,45 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
@@ -80,35 +127,45 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
}
// file, store in memory or on disk
@ -252,7 +315,7 @@ index fca5f9e15f..b0b4fb83a5 100644
}
form.File[name] = append(form.File[name], fh)
}
@@ -116,6 +198,17 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
@@ -116,6 +173,17 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
return form, nil
}
@ -270,7 +333,7 @@ index fca5f9e15f..b0b4fb83a5 100644
// Form is a parsed multipart form.
// Its File parts are stored either in memory or on disk,
// and are accessible via the *FileHeader's Open method.
@@ -133,7 +226,7 @@ func (f *Form) RemoveAll() error {
@@ -133,7 +201,7 @@ func (f *Form) RemoveAll() error {
for _, fh := range fhs {
if fh.tmpfile != "" {
e := os.Remove(fh.tmpfile)
@ -279,7 +342,7 @@ index fca5f9e15f..b0b4fb83a5 100644
err = e
}
}
@@ -148,15 +241,25 @@ type FileHeader struct {
@@ -148,15 +216,25 @@ type FileHeader struct {
Header textproto.MIMEHeader
Size int64
@ -308,7 +371,7 @@ index fca5f9e15f..b0b4fb83a5 100644
}
return os.Open(fh.tmpfile)
}
@@ -175,8 +278,12 @@ type File interface {
@@ -175,8 +253,12 @@ type File interface {
type sectionReadCloser struct {
*io.SectionReader
@ -322,7 +385,7 @@ index fca5f9e15f..b0b4fb83a5 100644
return nil
}
diff --git a/src/mime/multipart/formdata_test.go b/src/mime/multipart/formdata_test.go
index e3a3a3eae8..5cded7170c 100644
index e3a3a3e..5cded71 100644
--- a/src/mime/multipart/formdata_test.go
+++ b/src/mime/multipart/formdata_test.go
@@ -6,8 +6,10 @@ package multipart
@ -493,7 +556,7 @@ index e3a3a3eae8..5cded7170c 100644
+ }
+}
diff --git a/src/mime/multipart/multipart.go b/src/mime/multipart/multipart.go
index 81bf722d4e..465b376eae 100644
index 81bf722..465b376 100644
--- a/src/mime/multipart/multipart.go
+++ b/src/mime/multipart/multipart.go
@@ -128,12 +128,12 @@ func (r *stickyErrorReader) Read(p []byte) (n int, _ error) {
@ -583,7 +646,7 @@ index 81bf722d4e..465b376eae 100644
}
diff --git a/src/mime/multipart/readmimeheader.go b/src/mime/multipart/readmimeheader.go
new file mode 100644
index 0000000000..6836928c9e
index 0000000..6836928
--- /dev/null
+++ b/src/mime/multipart/readmimeheader.go
@@ -0,0 +1,14 @@
@ -602,7 +665,7 @@ index 0000000000..6836928c9e
+//go:linkname readMIMEHeader net/textproto.readMIMEHeader
+func readMIMEHeader(r *textproto.Reader, lim int64) (textproto.MIMEHeader, error)
diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go
index 4e0c4ba207..fac12b7ee6 100644
index 4e0c4ba..fac12b7 100644
--- a/src/net/http/request_test.go
+++ b/src/net/http/request_test.go
@@ -1110,7 +1110,7 @@ func testMissingFile(t *testing.T, req *Request) {
@ -615,7 +678,7 @@ index 4e0c4ba207..fac12b7ee6 100644
if err != ErrMissingFile {
t.Errorf("FormFile err = %q, want ErrMissingFile", err)
diff --git a/src/net/textproto/reader.go b/src/net/textproto/reader.go
index 5c3084f8a7..83db3b21ea 100644
index 5c3084f..83db3b2 100644
--- a/src/net/textproto/reader.go
+++ b/src/net/textproto/reader.go
@@ -7,8 +7,10 @@ package textproto

View File

@ -0,0 +1,197 @@
From da60f6100bbf663997609d8168af317dd32e83b9 Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Fri, 10 Mar 2023 14:21:05 -0800
Subject: [PATCH 1/6] [release-branch.go1.19] net/textproto: avoid
overpredicting the number of MIME header keys
A parsed MIME header is a map[string][]string. In the common case,
a header contains many one-element []string slices. To avoid
allocating a separate slice for each key, ReadMIMEHeader looks
ahead in the input to predict the number of keys that will be
parsed, and allocates a single []string of that length.
The individual slices are then allocated out of the larger one.
The prediction of the number of header keys was done by counting
newlines in the input buffer, which does not take into account
header continuation lines (where a header key/value spans multiple
lines) or the end of the header block and the start of the body.
This could lead to a substantial amount of overallocation, for
example when the body consists of nothing but a large block of
newlines.
Fix header key count prediction to take into account the end of
the headers (indicated by a blank line) and continuation lines
(starting with whitespace).
Thanks to Jakob Ackermann (@das7pad) for reporting this issue.
Fixes CVE-2023-24534
For #58975
Fixes #59267
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802452
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
(cherry picked from commit f739f080a72fd5b06d35c8e244165159645e2ed6)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802393
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Change-Id: I675451438d619a9130360c56daf529559004903f
Reviewed-on: https://go-review.googlesource.com/c/go/+/481982
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
---
src/net/textproto/reader.go | 30 ++++++++++------
src/net/textproto/reader_test.go | 59 ++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 11 deletions(-)
diff --git a/src/net/textproto/reader.go b/src/net/textproto/reader.go
index 83db3b21ea9..98056b0de55 100644
--- a/src/net/textproto/reader.go
+++ b/src/net/textproto/reader.go
@@ -493,8 +493,11 @@ func readMIMEHeader(r *Reader, lim int64) (MIMEHeader, error) {
// large one ahead of time which we'll cut up into smaller
// slices. If this isn't big enough later, we allocate small ones.
var strs []string
- hint := r.upcomingHeaderNewlines()
+ hint := r.upcomingHeaderKeys()
if hint > 0 {
+ if hint > 1000 {
+ hint = 1000 // set a cap to avoid overallocation
+ }
strs = make([]string, hint)
}
@@ -579,9 +582,11 @@ func mustHaveFieldNameColon(line []byte) error {
return nil
}
-// upcomingHeaderNewlines returns an approximation of the number of newlines
+var nl = []byte("\n")
+
+// upcomingHeaderKeys returns an approximation of the number of keys
// that will be in this header. If it gets confused, it returns 0.
-func (r *Reader) upcomingHeaderNewlines() (n int) {
+func (r *Reader) upcomingHeaderKeys() (n int) {
// Try to determine the 'hint' size.
r.R.Peek(1) // force a buffer load if empty
s := r.R.Buffered()
@@ -589,17 +594,20 @@ func (r *Reader) upcomingHeaderNewlines() (n int) {
return
}
peek, _ := r.R.Peek(s)
- for len(peek) > 0 {
- i := bytes.IndexByte(peek, '\n')
- if i < 3 {
- // Not present (-1) or found within the next few bytes,
- // implying we're at the end ("\r\n\r\n" or "\n\n")
- return
+ for len(peek) > 0 && n < 1000 {
+ var line []byte
+ line, peek, _ = bytes.Cut(peek, nl)
+ if len(line) == 0 || (len(line) == 1 && line[0] == '\r') {
+ // Blank line separating headers from the body.
+ break
+ }
+ if line[0] == ' ' || line[0] == '\t' {
+ // Folded continuation of the previous line.
+ continue
}
n++
- peek = peek[i+1:]
}
- return
+ return n
}
// CanonicalMIMEHeaderKey returns the canonical format of the
diff --git a/src/net/textproto/reader_test.go b/src/net/textproto/reader_test.go
index 3124d438fa5..3ae0de13530 100644
--- a/src/net/textproto/reader_test.go
+++ b/src/net/textproto/reader_test.go
@@ -9,6 +9,7 @@ import (
"bytes"
"io"
"reflect"
+ "runtime"
"strings"
"testing"
)
@@ -127,6 +128,42 @@ func TestReadMIMEHeaderSingle(t *testing.T) {
}
}
+// TestReaderUpcomingHeaderKeys is testing an internal function, but it's very
+// difficult to test well via the external API.
+func TestReaderUpcomingHeaderKeys(t *testing.T) {
+ for _, test := range []struct {
+ input string
+ want int
+ }{{
+ input: "",
+ want: 0,
+ }, {
+ input: "A: v",
+ want: 1,
+ }, {
+ input: "A: v\r\nB: v\r\n",
+ want: 2,
+ }, {
+ input: "A: v\nB: v\n",
+ want: 2,
+ }, {
+ input: "A: v\r\n continued\r\n still continued\r\nB: v\r\n\r\n",
+ want: 2,
+ }, {
+ input: "A: v\r\n\r\nB: v\r\nC: v\r\n",
+ want: 1,
+ }, {
+ input: "A: v" + strings.Repeat("\n", 1000),
+ want: 1,
+ }} {
+ r := reader(test.input)
+ got := r.upcomingHeaderKeys()
+ if test.want != got {
+ t.Fatalf("upcomingHeaderKeys(%q): %v; want %v", test.input, got, test.want)
+ }
+ }
+}
+
func TestReadMIMEHeaderNoKey(t *testing.T) {
r := reader(": bar\ntest-1: 1\n\n")
m, err := r.ReadMIMEHeader()
@@ -223,6 +260,28 @@ func TestReadMIMEHeaderTrimContinued(t *testing.T) {
}
}
+// Test that reading a header doesn't overallocate. Issue 58975.
+func TestReadMIMEHeaderAllocations(t *testing.T) {
+ var totalAlloc uint64
+ const count = 200
+ for i := 0; i < count; i++ {
+ r := reader("A: b\r\n\r\n" + strings.Repeat("\n", 4096))
+ var m1, m2 runtime.MemStats
+ runtime.ReadMemStats(&m1)
+ _, err := r.ReadMIMEHeader()
+ if err != nil {
+ t.Fatalf("ReadMIMEHeader: %v", err)
+ }
+ runtime.ReadMemStats(&m2)
+ totalAlloc += m2.TotalAlloc - m1.TotalAlloc
+ }
+ // 32k is large and we actually allocate substantially less,
+ // but prior to the fix for #58975 we allocated ~400k in this case.
+ if got, want := totalAlloc/count, uint64(32768); got > want {
+ t.Fatalf("ReadMIMEHeader allocated %v bytes, want < %v", got, want)
+ }
+}
+
type readResponseTest struct {
in string
inCode int
--
2.33.0

View File

@ -0,0 +1,91 @@
From bc1a60a500731fdd144e0fc5e1d750103819a74f Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Wed, 22 Mar 2023 09:33:22 -0700
Subject: [PATCH 2/6] [release-branch.go1.19] go/scanner: reject large line and
column numbers in //line directives
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
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>
---
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 1a46c878663..993df6315f1 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 f08e28cdd6b..2276e070498 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
}
@@ -267,7 +270,7 @@ func (s *Scanner) updateLineInfo(next, offs int, text []byte) {
line = n
}
- if line == 0 {
+ if line == 0 || line > maxLineCol {
s.error(offs+i, "invalid line number: "+string(text[i:]))
return
}
--
2.33.0

View File

@ -0,0 +1,378 @@
From d406fb465e0f83d26c02d7ffd9e16b9a0a504ae0 Mon Sep 17 00:00:00 2001
From: Roland Shoemaker <bracewell@google.com>
Date: Mon, 20 Mar 2023 11:01:13 -0700
Subject: [PATCH 3/6] [release-branch.go1.19] html/template: disallow actions
in JS template literals
ECMAScript 6 introduced template literals[0][1] which are delimited with
backticks. These need to be escaped in a similar fashion to the
delimiters for other string literals. Additionally template literals can
contain special syntax for string interpolation.
There is no clear way to allow safe insertion of actions within JS
template literals, as handling (JS) string interpolation inside of these
literals is rather complex. As such we've chosen to simply disallow
template actions within these template literals.
A new error code is added for this parsing failure case, errJsTmplLit,
but it is unexported as it is not backwards compatible with other minor
release versions to introduce an API change in a minor release. We will
export this code in the next major release.
The previous behavior (with the cavet that backticks are now escaped
properly) can be re-enabled with GODEBUG=jstmpllitinterp=1.
This change subsumes CL471455.
Thanks to Sohom Datta, Manipal Institute of Technology, for reporting
this issue.
Fixes CVE-2023-24538
For #59234
Fixes #59271
[0] https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-template-literals
[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802457
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802612
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Change-Id: Ic7f10595615f2b2740d9c85ad7ef40dc0e78c04c
Reviewed-on: https://go-review.googlesource.com/c/go/+/481987
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
---
src/html/template/context.go | 4 ++
src/html/template/error.go | 13 ++++++
src/html/template/escape.go | 11 ++++++
src/html/template/escape_test.go | 66 +++++++++++++++++--------------
src/html/template/js.go | 2 +
src/html/template/js_test.go | 2 +-
src/html/template/jsctx_string.go | 9 +++++
src/html/template/state_string.go | 37 ++++++++++++++++-
src/html/template/transition.go | 7 +++-
9 files changed, 118 insertions(+), 33 deletions(-)
diff --git a/src/html/template/context.go b/src/html/template/context.go
index f7d4849928d..a67b5a78712 100644
--- a/src/html/template/context.go
+++ b/src/html/template/context.go
@@ -116,6 +116,8 @@ const (
stateJSDqStr
// stateJSSqStr occurs inside a JavaScript single quoted string.
stateJSSqStr
+ // stateJSBqStr occurs inside a JavaScript back quoted string.
+ stateJSBqStr
// stateJSRegexp occurs inside a JavaScript regexp literal.
stateJSRegexp
// stateJSBlockCmt occurs inside a JavaScript /* block comment */.
@@ -141,6 +143,8 @@ const (
// stateError is an infectious error state outside any valid
// HTML/CSS/JS construct.
stateError
+ // stateDead marks unreachable code after a {{break}} or {{continue}}.
+ stateDead
)
// isComment is true for any state that contains content meant for template
diff --git a/src/html/template/error.go b/src/html/template/error.go
index 0e527063ea6..fd26b64b6e9 100644
--- a/src/html/template/error.go
+++ b/src/html/template/error.go
@@ -211,6 +211,19 @@ const (
// pipeline occurs in an unquoted attribute value context, "html" is
// disallowed. Avoid using "html" and "urlquery" entirely in new templates.
ErrPredefinedEscaper
+
+ // errJSTmplLit: "... appears in a JS template literal"
+ // Example:
+ // <script>var tmpl = `{{.Interp}`</script>
+ // Discussion:
+ // Package html/template does not support actions inside of JS template
+ // literals.
+ //
+ // TODO(rolandshoemaker): we cannot add this as an exported error in a minor
+ // release, since it is backwards incompatible with the other minor
+ // releases. As such we need to leave it unexported, and then we'll add it
+ // in the next major release.
+ errJSTmplLit
)
func (e *Error) Error() string {
diff --git a/src/html/template/escape.go b/src/html/template/escape.go
index 8739735cb7b..ca078f40ead 100644
--- a/src/html/template/escape.go
+++ b/src/html/template/escape.go
@@ -8,6 +8,7 @@ import (
"bytes"
"fmt"
"html"
+ "internal/godebug"
"io"
"text/template"
"text/template/parse"
@@ -205,6 +206,16 @@ func (e *escaper) escapeAction(c context, n *parse.ActionNode) context {
c.jsCtx = jsCtxDivOp
case stateJSDqStr, stateJSSqStr:
s = append(s, "_html_template_jsstrescaper")
+ case stateJSBqStr:
+ debugAllowActionJSTmpl := godebug.Get("jstmpllitinterp")
+ if debugAllowActionJSTmpl == "1" {
+ s = append(s, "_html_template_jsstrescaper")
+ } else {
+ return context{
+ state: stateError,
+ err: errorf(errJSTmplLit, n, n.Line, "%s appears in a JS template literal", n),
+ }
+ }
case stateJSRegexp:
s = append(s, "_html_template_jsregexpescaper")
case stateCSS:
diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go
index fbc84a75928..9d7749fc9ca 100644
--- a/src/html/template/escape_test.go
+++ b/src/html/template/escape_test.go
@@ -681,35 +681,31 @@ func TestEscape(t *testing.T) {
}
for _, test := range tests {
- tmpl := New(test.name)
- tmpl = Must(tmpl.Parse(test.input))
- // Check for bug 6459: Tree field was not set in Parse.
- if tmpl.Tree != tmpl.text.Tree {
- t.Errorf("%s: tree not set properly", test.name)
- continue
- }
- b := new(bytes.Buffer)
- if err := tmpl.Execute(b, data); err != nil {
- t.Errorf("%s: template execution failed: %s", test.name, err)
- continue
- }
- if w, g := test.output, b.String(); w != g {
- t.Errorf("%s: escaped output: want\n\t%q\ngot\n\t%q", test.name, w, g)
- continue
- }
- b.Reset()
- if err := tmpl.Execute(b, pdata); err != nil {
- t.Errorf("%s: template execution failed for pointer: %s", test.name, err)
- continue
- }
- if w, g := test.output, b.String(); w != g {
- t.Errorf("%s: escaped output for pointer: want\n\t%q\ngot\n\t%q", test.name, w, g)
- continue
- }
- if tmpl.Tree != tmpl.text.Tree {
- t.Errorf("%s: tree mismatch", test.name)
- continue
- }
+ t.Run(test.name, func(t *testing.T) {
+ tmpl := New(test.name)
+ tmpl = Must(tmpl.Parse(test.input))
+ // Check for bug 6459: Tree field was not set in Parse.
+ if tmpl.Tree != tmpl.text.Tree {
+ t.Fatalf("%s: tree not set properly", test.name)
+ }
+ b := new(strings.Builder)
+ if err := tmpl.Execute(b, data); err != nil {
+ t.Fatalf("%s: template execution failed: %s", test.name, err)
+ }
+ if w, g := test.output, b.String(); w != g {
+ t.Fatalf("%s: escaped output: want\n\t%q\ngot\n\t%q", test.name, w, g)
+ }
+ b.Reset()
+ if err := tmpl.Execute(b, pdata); err != nil {
+ t.Fatalf("%s: template execution failed for pointer: %s", test.name, err)
+ }
+ if w, g := test.output, b.String(); w != g {
+ t.Fatalf("%s: escaped output for pointer: want\n\t%q\ngot\n\t%q", test.name, w, g)
+ }
+ if tmpl.Tree != tmpl.text.Tree {
+ t.Fatalf("%s: tree mismatch", test.name)
+ }
+ })
}
}
@@ -920,6 +916,10 @@ func TestErrors(t *testing.T) {
"<a href='/foo?{{range .Items}}&{{.K}}={{.V}}{{end}}'>",
"",
},
+ {
+ "<script>var a = `${a+b}`</script>`",
+ "",
+ },
// Error cases.
{
"{{if .Cond}}<a{{end}}",
@@ -1058,6 +1058,10 @@ func TestErrors(t *testing.T) {
// html is allowed since it is the last command in the pipeline, but urlquery is not.
`predefined escaper "urlquery" disallowed in template`,
},
+ {
+ "<script>var tmpl = `asd {{.}}`;</script>",
+ `{{.}} appears in a JS template literal`,
+ },
}
for _, test := range tests {
buf := new(bytes.Buffer)
@@ -1279,6 +1283,10 @@ func TestEscapeText(t *testing.T) {
`<a onclick="'foo&quot;`,
context{state: stateJSSqStr, delim: delimDoubleQuote, attr: attrScript},
},
+ {
+ "<a onclick=\"`foo",
+ context{state: stateJSBqStr, delim: delimDoubleQuote, attr: attrScript},
+ },
{
`<A ONCLICK="'`,
context{state: stateJSSqStr, delim: delimDoubleQuote, attr: attrScript},
diff --git a/src/html/template/js.go b/src/html/template/js.go
index ea9c18346ba..b888eaf8b73 100644
--- a/src/html/template/js.go
+++ b/src/html/template/js.go
@@ -308,6 +308,7 @@ var jsStrReplacementTable = []string{
// Encode HTML specials as hex so the output can be embedded
// in HTML attributes without further encoding.
'"': `\u0022`,
+ '`': `\u0060`,
'&': `\u0026`,
'\'': `\u0027`,
'+': `\u002b`,
@@ -331,6 +332,7 @@ var jsStrNormReplacementTable = []string{
'"': `\u0022`,
'&': `\u0026`,
'\'': `\u0027`,
+ '`': `\u0060`,
'+': `\u002b`,
'/': `\/`,
'<': `\u003c`,
diff --git a/src/html/template/js_test.go b/src/html/template/js_test.go
index d7ee47b87d4..7d963ae6f15 100644
--- a/src/html/template/js_test.go
+++ b/src/html/template/js_test.go
@@ -292,7 +292,7 @@ func TestEscapersOnLower7AndSelectHighCodepoints(t *testing.T) {
`0123456789:;\u003c=\u003e?` +
`@ABCDEFGHIJKLMNO` +
`PQRSTUVWXYZ[\\]^_` +
- "`abcdefghijklmno" +
+ "\\u0060abcdefghijklmno" +
"pqrstuvwxyz{|}~\u007f" +
"\u00A0\u0100\\u2028\\u2029\ufeff\U0001D11E",
},
diff --git a/src/html/template/jsctx_string.go b/src/html/template/jsctx_string.go
index dd1d87ee454..23948934c95 100644
--- a/src/html/template/jsctx_string.go
+++ b/src/html/template/jsctx_string.go
@@ -4,6 +4,15 @@ package template
import "strconv"
+func _() {
+ // An "invalid array index" compiler error signifies that the constant values have changed.
+ // Re-run the stringer command to generate them again.
+ var x [1]struct{}
+ _ = x[jsCtxRegexp-0]
+ _ = x[jsCtxDivOp-1]
+ _ = x[jsCtxUnknown-2]
+}
+
const _jsCtx_name = "jsCtxRegexpjsCtxDivOpjsCtxUnknown"
var _jsCtx_index = [...]uint8{0, 11, 21, 33}
diff --git a/src/html/template/state_string.go b/src/html/template/state_string.go
index 05104be89c2..6fb1a6eeb0e 100644
--- a/src/html/template/state_string.go
+++ b/src/html/template/state_string.go
@@ -4,9 +4,42 @@ package template
import "strconv"
-const _state_name = "stateTextstateTagstateAttrNamestateAfterNamestateBeforeValuestateHTMLCmtstateRCDATAstateAttrstateURLstateSrcsetstateJSstateJSDqStrstateJSSqStrstateJSRegexpstateJSBlockCmtstateJSLineCmtstateCSSstateCSSDqStrstateCSSSqStrstateCSSDqURLstateCSSSqURLstateCSSURLstateCSSBlockCmtstateCSSLineCmtstateError"
+func _() {
+ // An "invalid array index" compiler error signifies that the constant values have changed.
+ // Re-run the stringer command to generate them again.
+ var x [1]struct{}
+ _ = x[stateText-0]
+ _ = x[stateTag-1]
+ _ = x[stateAttrName-2]
+ _ = x[stateAfterName-3]
+ _ = x[stateBeforeValue-4]
+ _ = x[stateHTMLCmt-5]
+ _ = x[stateRCDATA-6]
+ _ = x[stateAttr-7]
+ _ = x[stateURL-8]
+ _ = x[stateSrcset-9]
+ _ = x[stateJS-10]
+ _ = x[stateJSDqStr-11]
+ _ = x[stateJSSqStr-12]
+ _ = x[stateJSBqStr-13]
+ _ = x[stateJSRegexp-14]
+ _ = x[stateJSBlockCmt-15]
+ _ = x[stateJSLineCmt-16]
+ _ = x[stateCSS-17]
+ _ = x[stateCSSDqStr-18]
+ _ = x[stateCSSSqStr-19]
+ _ = x[stateCSSDqURL-20]
+ _ = x[stateCSSSqURL-21]
+ _ = x[stateCSSURL-22]
+ _ = x[stateCSSBlockCmt-23]
+ _ = x[stateCSSLineCmt-24]
+ _ = x[stateError-25]
+ _ = x[stateDead-26]
+}
+
+const _state_name = "stateTextstateTagstateAttrNamestateAfterNamestateBeforeValuestateHTMLCmtstateRCDATAstateAttrstateURLstateSrcsetstateJSstateJSDqStrstateJSSqStrstateJSBqStrstateJSRegexpstateJSBlockCmtstateJSLineCmtstateCSSstateCSSDqStrstateCSSSqStrstateCSSDqURLstateCSSSqURLstateCSSURLstateCSSBlockCmtstateCSSLineCmtstateErrorstateDead"
-var _state_index = [...]uint16{0, 9, 17, 30, 44, 60, 72, 83, 92, 100, 111, 118, 130, 142, 155, 170, 184, 192, 205, 218, 231, 244, 255, 271, 286, 296}
+var _state_index = [...]uint16{0, 9, 17, 30, 44, 60, 72, 83, 92, 100, 111, 118, 130, 142, 154, 167, 182, 196, 204, 217, 230, 243, 256, 267, 283, 298, 308, 317}
func (i state) String() string {
if i >= state(len(_state_index)-1) {
diff --git a/src/html/template/transition.go b/src/html/template/transition.go
index 06df679330d..92eb3519063 100644
--- a/src/html/template/transition.go
+++ b/src/html/template/transition.go
@@ -27,6 +27,7 @@ var transitionFunc = [...]func(context, []byte) (context, int){
stateJS: tJS,
stateJSDqStr: tJSDelimited,
stateJSSqStr: tJSDelimited,
+ stateJSBqStr: tJSDelimited,
stateJSRegexp: tJSDelimited,
stateJSBlockCmt: tBlockCmt,
stateJSLineCmt: tLineCmt,
@@ -262,7 +263,7 @@ func tURL(c context, s []byte) (context, int) {
// tJS is the context transition function for the JS state.
func tJS(c context, s []byte) (context, int) {
- i := bytes.IndexAny(s, `"'/`)
+ i := bytes.IndexAny(s, "\"`'/")
if i == -1 {
// Entire input is non string, comment, regexp tokens.
c.jsCtx = nextJSCtx(s, c.jsCtx)
@@ -274,6 +275,8 @@ func tJS(c context, s []byte) (context, int) {
c.state, c.jsCtx = stateJSDqStr, jsCtxRegexp
case '\'':
c.state, c.jsCtx = stateJSSqStr, jsCtxRegexp
+ case '`':
+ c.state, c.jsCtx = stateJSBqStr, jsCtxRegexp
case '/':
switch {
case i+1 < len(s) && s[i+1] == '/':
@@ -303,6 +306,8 @@ func tJSDelimited(c context, s []byte) (context, int) {
switch c.state {
case stateJSSqStr:
specials = `\'`
+ case stateJSBqStr:
+ specials = "`\\"
case stateJSRegexp:
specials = `\/[]`
}
--
2.33.0

View File

@ -0,0 +1,133 @@
From 3c219fd3bda683ee4bc5b69800032ed876b053a9 Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Thu, 16 Mar 2023 14:18:04 -0700
Subject: [PATCH 4/6] [release-branch.go1.19] mime/multipart: avoid excessive
copy buffer allocations in ReadForm
When copying form data to disk with io.Copy,
allocate only one copy buffer and reuse it rather than
creating two buffers per file (one from io.multiReader.WriteTo,
and a second one from os.File.ReadFrom).
Thanks to Jakob Ackermann (@das7pad) for reporting this issue.
For CVE-2023-24536
For #59153
For #59269
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802453
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802395
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Change-Id: Ie405470c92abffed3356913b37d813e982c96c8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/481983
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
---
src/mime/multipart/formdata.go | 15 +++++++--
src/mime/multipart/formdata_test.go | 49 +++++++++++++++++++++++++++++
2 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/src/mime/multipart/formdata.go b/src/mime/multipart/formdata.go
index a7d4ca97f04..975dcb6b26d 100644
--- a/src/mime/multipart/formdata.go
+++ b/src/mime/multipart/formdata.go
@@ -84,6 +84,7 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
maxMemoryBytes = math.MaxInt64
}
}
+ var copyBuf []byte
for {
p, err := r.nextPart(false, maxMemoryBytes)
if err == io.EOF {
@@ -147,14 +148,22 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
}
}
numDiskFiles++
- size, err := io.Copy(file, io.MultiReader(&b, p))
+ if _, err := file.Write(b.Bytes()); err != nil {
+ return nil, err
+ }
+ if copyBuf == nil {
+ copyBuf = make([]byte, 32*1024) // same buffer size as io.Copy uses
+ }
+ // os.File.ReadFrom will allocate its own copy buffer if we let io.Copy use it.
+ type writerOnly struct{ io.Writer }
+ remainingSize, err := io.CopyBuffer(writerOnly{file}, p, copyBuf)
if err != nil {
return nil, err
}
fh.tmpfile = file.Name()
- fh.Size = size
+ fh.Size = int64(b.Len()) + remainingSize
fh.tmpoff = fileOff
- fileOff += size
+ fileOff += fh.Size
if !combineFiles {
if err := file.Close(); err != nil {
return nil, err
diff --git a/src/mime/multipart/formdata_test.go b/src/mime/multipart/formdata_test.go
index 5cded7170c6..f5b56083b23 100644
--- a/src/mime/multipart/formdata_test.go
+++ b/src/mime/multipart/formdata_test.go
@@ -368,3 +368,52 @@ func testReadFormManyFiles(t *testing.T, distinct bool) {
t.Fatalf("temp dir contains %v files; want 0", len(names))
}
}
+
+func BenchmarkReadForm(b *testing.B) {
+ for _, test := range []struct {
+ name string
+ form func(fw *Writer, count int)
+ }{{
+ name: "fields",
+ form: func(fw *Writer, count int) {
+ for i := 0; i < count; i++ {
+ w, _ := fw.CreateFormField(fmt.Sprintf("field%v", i))
+ fmt.Fprintf(w, "value %v", i)
+ }
+ },
+ }, {
+ name: "files",
+ form: func(fw *Writer, count int) {
+ for i := 0; i < count; i++ {
+ w, _ := fw.CreateFormFile(fmt.Sprintf("field%v", i), fmt.Sprintf("file%v", i))
+ fmt.Fprintf(w, "value %v", i)
+ }
+ },
+ }} {
+ b.Run(test.name, func(b *testing.B) {
+ for _, maxMemory := range []int64{
+ 0,
+ 1 << 20,
+ } {
+ var buf bytes.Buffer
+ fw := NewWriter(&buf)
+ test.form(fw, 10)
+ if err := fw.Close(); err != nil {
+ b.Fatal(err)
+ }
+ b.Run(fmt.Sprintf("maxMemory=%v", maxMemory), func(b *testing.B) {
+ b.ReportAllocs()
+ for i := 0; i < b.N; i++ {
+ fr := NewReader(bytes.NewReader(buf.Bytes()), fw.Boundary())
+ form, err := fr.ReadForm(maxMemory)
+ if err != nil {
+ b.Fatal(err)
+ }
+ form.RemoveAll()
+ }
+
+ })
+ }
+ })
+ }
+}
--
2.33.0

View File

@ -0,0 +1,183 @@
From add85fa4e76febeae7be4484a4d62bf647854244 Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Thu, 16 Mar 2023 16:56:12 -0700
Subject: [PATCH 5/6] [release-branch.go1.19] net/textproto, mime/multipart:
improve accounting of non-file data
For requests containing large numbers of small parts,
memory consumption of a parsed form could be about 250%
over the estimated size.
When considering the size of parsed forms, account for the size of
FileHeader structs and increase the estimate of memory consumed by
map entries.
Thanks to Jakob Ackermann (@das7pad) for reporting this issue.
For CVE-2023-24536
For #59153
For #59269
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802454
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802396
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Change-Id: I31bc50e9346b4eee6fbe51a18c3c57230cc066db
Reviewed-on: https://go-review.googlesource.com/c/go/+/481984
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
---
src/mime/multipart/formdata.go | 9 +++--
src/mime/multipart/formdata_test.go | 55 ++++++++++++-----------------
src/net/textproto/reader.go | 8 ++++-
3 files changed, 37 insertions(+), 35 deletions(-)
diff --git a/src/mime/multipart/formdata.go b/src/mime/multipart/formdata.go
index 975dcb6b26d..3f6ff697ca6 100644
--- a/src/mime/multipart/formdata.go
+++ b/src/mime/multipart/formdata.go
@@ -103,8 +103,9 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
// Multiple values for the same key (one map entry, longer slice) are cheaper
// than the same number of values for different keys (many map entries), but
// using a consistent per-value cost for overhead is simpler.
+ const mapEntryOverhead = 200
maxMemoryBytes -= int64(len(name))
- maxMemoryBytes -= 100 // map overhead
+ maxMemoryBytes -= mapEntryOverhead
if maxMemoryBytes < 0 {
// We can't actually take this path, since nextPart would already have
// rejected the MIME headers for being too large. Check anyway.
@@ -128,7 +129,10 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
}
// file, store in memory or on disk
+ const fileHeaderSize = 100
maxMemoryBytes -= mimeHeaderSize(p.Header)
+ maxMemoryBytes -= mapEntryOverhead
+ maxMemoryBytes -= fileHeaderSize
if maxMemoryBytes < 0 {
return nil, ErrMessageTooLarge
}
@@ -183,9 +187,10 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
}
func mimeHeaderSize(h textproto.MIMEHeader) (size int64) {
+ size = 400
for k, vs := range h {
size += int64(len(k))
- size += 100 // map entry overhead
+ size += 200 // map entry overhead
for _, v := range vs {
size += int64(len(v))
}
diff --git a/src/mime/multipart/formdata_test.go b/src/mime/multipart/formdata_test.go
index f5b56083b23..8ed26e0c340 100644
--- a/src/mime/multipart/formdata_test.go
+++ b/src/mime/multipart/formdata_test.go
@@ -192,10 +192,10 @@ func (r *failOnReadAfterErrorReader) Read(p []byte) (n int, err error) {
// TestReadForm_NonFileMaxMemory asserts that the ReadForm maxMemory limit is applied
// while processing non-file form data as well as file form data.
func TestReadForm_NonFileMaxMemory(t *testing.T) {
- n := 10<<20 + 25
if testing.Short() {
- n = 10<<10 + 25
+ t.Skip("skipping in -short mode")
}
+ n := 10 << 20
largeTextValue := strings.Repeat("1", n)
message := `--MyBoundary
Content-Disposition: form-data; name="largetext"
@@ -203,38 +203,29 @@ Content-Disposition: form-data; name="largetext"
` + largeTextValue + `
--MyBoundary--
`
-
testBody := strings.ReplaceAll(message, "\n", "\r\n")
- testCases := []struct {
- name string
- maxMemory int64
- err error
- }{
- {"smaller", 50 + int64(len("largetext")) + 100, nil},
- {"exact-fit", 25 + int64(len("largetext")) + 100, nil},
- {"too-large", 0, ErrMessageTooLarge},
- }
- for _, tc := range testCases {
- t.Run(tc.name, func(t *testing.T) {
- if tc.maxMemory == 0 && testing.Short() {
- t.Skip("skipping in -short mode")
- }
- b := strings.NewReader(testBody)
- r := NewReader(b, boundary)
- f, err := r.ReadForm(tc.maxMemory)
- if err == nil {
- defer f.RemoveAll()
- }
- if tc.err != err {
- t.Fatalf("ReadForm error - got: %v; expected: %v", err, tc.err)
- }
- if err == nil {
- if g := f.Value["largetext"][0]; g != largeTextValue {
- t.Errorf("largetext mismatch: got size: %v, expected size: %v", len(g), len(largeTextValue))
- }
- }
- })
+ // Try parsing the form with increasing maxMemory values.
+ // Changes in how we account for non-file form data may cause the exact point
+ // where we change from rejecting the form as too large to accepting it to vary,
+ // but we should see both successes and failures.
+ const failWhenMaxMemoryLessThan = 128
+ for maxMemory := int64(0); maxMemory < failWhenMaxMemoryLessThan*2; maxMemory += 16 {
+ b := strings.NewReader(testBody)
+ r := NewReader(b, boundary)
+ f, err := r.ReadForm(maxMemory)
+ if err != nil {
+ continue
+ }
+ if g := f.Value["largetext"][0]; g != largeTextValue {
+ t.Errorf("largetext mismatch: got size: %v, expected size: %v", len(g), len(largeTextValue))
+ }
+ f.RemoveAll()
+ if maxMemory < failWhenMaxMemoryLessThan {
+ t.Errorf("ReadForm(%v): no error, expect to hit memory limit when maxMemory < %v", maxMemory, failWhenMaxMemoryLessThan)
+ }
+ return
}
+ t.Errorf("ReadForm(x) failed for x < 1024, expect success")
}
// TestReadForm_MetadataTooLarge verifies that we account for the size of field names,
diff --git a/src/net/textproto/reader.go b/src/net/textproto/reader.go
index 98056b0de55..68b445083bf 100644
--- a/src/net/textproto/reader.go
+++ b/src/net/textproto/reader.go
@@ -503,6 +503,12 @@ func readMIMEHeader(r *Reader, lim int64) (MIMEHeader, error) {
m := make(MIMEHeader, hint)
+ // Account for 400 bytes of overhead for the MIMEHeader, plus 200 bytes per entry.
+ // Benchmarking map creation as of go1.20, a one-entry MIMEHeader is 416 bytes and large
+ // MIMEHeaders average about 200 bytes per entry.
+ lim -= 400
+ const mapEntryOverhead = 200
+
// The first line cannot start with a leading space.
if buf, err := r.R.Peek(1); err == nil && (buf[0] == ' ' || buf[0] == '\t') {
line, err := r.readLineSlice()
@@ -542,7 +548,7 @@ func readMIMEHeader(r *Reader, lim int64) (MIMEHeader, error) {
vv := m[key]
if vv == nil {
lim -= int64(len(key))
- lim -= 100 // map entry overhead
+ lim -= mapEntryOverhead
}
lim -= int64(len(value))
if lim < 0 {
--
2.33.0

View File

@ -0,0 +1,346 @@
From fd764bb32d4be31e497a3279977ca760305c501b Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Mon, 20 Mar 2023 10:43:19 -0700
Subject: [PATCH 6/6] [release-branch.go1.19] mime/multipart: limit parsed mime
message sizes
The parsed forms of MIME headers and multipart forms can consume
substantially more memory than the size of the input data.
A malicious input containing a very large number of headers or
form parts can cause excessively large memory allocations.
Set limits on the size of MIME data:
Reader.NextPart and Reader.NextRawPart limit the the number
of headers in a part to 10000.
Reader.ReadForm limits the total number of headers in all
FileHeaders to 10000.
Both of these limits may be set with with
GODEBUG=multipartmaxheaders=<values>.
Reader.ReadForm limits the number of parts in a form to 1000.
This limit may be set with GODEBUG=multipartmaxparts=<value>.
Thanks for Jakob Ackermann (@das7pad) for reporting this issue.
For CVE-2023-24536
For #59153
For #59269
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802455
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1801087
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Change-Id: If134890d75f0d95c681d67234daf191ba08e6424
Reviewed-on: https://go-review.googlesource.com/c/go/+/481985
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
---
src/mime/multipart/formdata.go | 19 ++++++++-
src/mime/multipart/formdata_test.go | 61 ++++++++++++++++++++++++++++
src/mime/multipart/multipart.go | 31 ++++++++++----
src/mime/multipart/readmimeheader.go | 2 +-
src/net/textproto/reader.go | 19 +++++----
5 files changed, 115 insertions(+), 17 deletions(-)
diff --git a/src/mime/multipart/formdata.go b/src/mime/multipart/formdata.go
index 3f6ff697ca6..4f26aab2cf4 100644
--- a/src/mime/multipart/formdata.go
+++ b/src/mime/multipart/formdata.go
@@ -12,6 +12,7 @@ import (
"math"
"net/textproto"
"os"
+ "strconv"
)
// ErrMessageTooLarge is returned by ReadForm if the message form
@@ -41,6 +42,15 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
numDiskFiles := 0
multipartFiles := godebug.Get("multipartfiles")
combineFiles := multipartFiles != "distinct"
+ maxParts := 1000
+ multipartMaxParts := godebug.Get("multipartmaxparts")
+ if multipartMaxParts != "" {
+ if v, err := strconv.Atoi(multipartMaxParts); err == nil && v >= 0 {
+ maxParts = v
+ }
+ }
+ maxHeaders := maxMIMEHeaders()
+
defer func() {
if file != nil {
if cerr := file.Close(); err == nil {
@@ -86,13 +96,17 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
}
var copyBuf []byte
for {
- p, err := r.nextPart(false, maxMemoryBytes)
+ p, err := r.nextPart(false, maxMemoryBytes, maxHeaders)
if err == io.EOF {
break
}
if err != nil {
return nil, err
}
+ if maxParts <= 0 {
+ return nil, ErrMessageTooLarge
+ }
+ maxParts--
name := p.FormName()
if name == "" {
@@ -136,6 +150,9 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
if maxMemoryBytes < 0 {
return nil, ErrMessageTooLarge
}
+ for _, v := range p.Header {
+ maxHeaders -= int64(len(v))
+ }
fh := &FileHeader{
Filename: filename,
Header: p.Header,
diff --git a/src/mime/multipart/formdata_test.go b/src/mime/multipart/formdata_test.go
index 8ed26e0c340..c78eeb7a12b 100644
--- a/src/mime/multipart/formdata_test.go
+++ b/src/mime/multipart/formdata_test.go
@@ -360,6 +360,67 @@ func testReadFormManyFiles(t *testing.T, distinct bool) {
}
}
+func TestReadFormLimits(t *testing.T) {
+ for _, test := range []struct {
+ values int
+ files int
+ extraKeysPerFile int
+ wantErr error
+ godebug string
+ }{
+ {values: 1000},
+ {values: 1001, wantErr: ErrMessageTooLarge},
+ {values: 500, files: 500},
+ {values: 501, files: 500, wantErr: ErrMessageTooLarge},
+ {files: 1000},
+ {files: 1001, wantErr: ErrMessageTooLarge},
+ {files: 1, extraKeysPerFile: 9998}, // plus Content-Disposition and Content-Type
+ {files: 1, extraKeysPerFile: 10000, wantErr: ErrMessageTooLarge},
+ {godebug: "multipartmaxparts=100", values: 100},
+ {godebug: "multipartmaxparts=100", values: 101, wantErr: ErrMessageTooLarge},
+ {godebug: "multipartmaxheaders=100", files: 2, extraKeysPerFile: 48},
+ {godebug: "multipartmaxheaders=100", files: 2, extraKeysPerFile: 50, wantErr: ErrMessageTooLarge},
+ } {
+ name := fmt.Sprintf("values=%v/files=%v/extraKeysPerFile=%v", test.values, test.files, test.extraKeysPerFile)
+ if test.godebug != "" {
+ name += fmt.Sprintf("/godebug=%v", test.godebug)
+ }
+ t.Run(name, func(t *testing.T) {
+ if test.godebug != "" {
+ t.Setenv("GODEBUG", test.godebug)
+ }
+ var buf bytes.Buffer
+ fw := NewWriter(&buf)
+ for i := 0; i < test.values; i++ {
+ w, _ := fw.CreateFormField(fmt.Sprintf("field%v", i))
+ fmt.Fprintf(w, "value %v", i)
+ }
+ for i := 0; i < test.files; i++ {
+ h := make(textproto.MIMEHeader)
+ h.Set("Content-Disposition",
+ fmt.Sprintf(`form-data; name="file%v"; filename="file%v"`, i, i))
+ h.Set("Content-Type", "application/octet-stream")
+ for j := 0; j < test.extraKeysPerFile; j++ {
+ h.Set(fmt.Sprintf("k%v", j), "v")
+ }
+ w, _ := fw.CreatePart(h)
+ fmt.Fprintf(w, "value %v", i)
+ }
+ if err := fw.Close(); err != nil {
+ t.Fatal(err)
+ }
+ fr := NewReader(bytes.NewReader(buf.Bytes()), fw.Boundary())
+ form, err := fr.ReadForm(1 << 10)
+ if err == nil {
+ defer form.RemoveAll()
+ }
+ if err != test.wantErr {
+ t.Errorf("ReadForm = %v, want %v", err, test.wantErr)
+ }
+ })
+ }
+}
+
func BenchmarkReadForm(b *testing.B) {
for _, test := range []struct {
name string
diff --git a/src/mime/multipart/multipart.go b/src/mime/multipart/multipart.go
index 465b376eaeb..bc5096f018f 100644
--- a/src/mime/multipart/multipart.go
+++ b/src/mime/multipart/multipart.go
@@ -16,11 +16,13 @@ import (
"bufio"
"bytes"
"fmt"
+ "internal/godebug"
"io"
"mime"
"mime/quotedprintable"
"net/textproto"
"path/filepath"
+ "strconv"
"strings"
)
@@ -128,12 +130,12 @@ func (r *stickyErrorReader) Read(p []byte) (n int, _ error) {
return n, r.err
}
-func newPart(mr *Reader, rawPart bool, maxMIMEHeaderSize int64) (*Part, error) {
+func newPart(mr *Reader, rawPart bool, maxMIMEHeaderSize, maxMIMEHeaders int64) (*Part, error) {
bp := &Part{
Header: make(map[string][]string),
mr: mr,
}
- if err := bp.populateHeaders(maxMIMEHeaderSize); err != nil {
+ if err := bp.populateHeaders(maxMIMEHeaderSize, maxMIMEHeaders); err != nil {
return nil, err
}
bp.r = partReader{bp}
@@ -149,9 +151,9 @@ func newPart(mr *Reader, rawPart bool, maxMIMEHeaderSize int64) (*Part, error) {
return bp, nil
}
-func (p *Part) populateHeaders(maxMIMEHeaderSize int64) error {
+func (p *Part) populateHeaders(maxMIMEHeaderSize, maxMIMEHeaders int64) error {
r := textproto.NewReader(p.mr.bufReader)
- header, err := readMIMEHeader(r, maxMIMEHeaderSize)
+ header, err := readMIMEHeader(r, maxMIMEHeaderSize, maxMIMEHeaders)
if err == nil {
p.Header = header
}
@@ -313,6 +315,19 @@ type Reader struct {
// including header keys, values, and map overhead.
const maxMIMEHeaderSize = 10 << 20
+func maxMIMEHeaders() int64 {
+ // multipartMaxHeaders is the maximum number of header entries NextPart will return,
+ // as well as the maximum combined total of header entries Reader.ReadForm will return
+ // in FileHeaders.
+ multipartMaxHeaders := godebug.Get("multipartmaxheaders")
+ if multipartMaxHeaders != "" {
+ if v, err := strconv.ParseInt(multipartMaxHeaders, 10, 64); err == nil && v >= 0 {
+ return v
+ }
+ }
+ return 10000
+}
+
// NextPart returns the next part in the multipart or an error.
// When there are no more parts, the error io.EOF is returned.
//
@@ -320,7 +335,7 @@ const maxMIMEHeaderSize = 10 << 20
// has a value of "quoted-printable", that header is instead
// hidden and the body is transparently decoded during Read calls.
func (r *Reader) NextPart() (*Part, error) {
- return r.nextPart(false, maxMIMEHeaderSize)
+ return r.nextPart(false, maxMIMEHeaderSize, maxMIMEHeaders())
}
// NextRawPart returns the next part in the multipart or an error.
@@ -329,10 +344,10 @@ func (r *Reader) NextPart() (*Part, error) {
// Unlike NextPart, it does not have special handling for
// "Content-Transfer-Encoding: quoted-printable".
func (r *Reader) NextRawPart() (*Part, error) {
- return r.nextPart(true, maxMIMEHeaderSize)
+ return r.nextPart(true, maxMIMEHeaderSize, maxMIMEHeaders())
}
-func (r *Reader) nextPart(rawPart bool, maxMIMEHeaderSize int64) (*Part, error) {
+func (r *Reader) nextPart(rawPart bool, maxMIMEHeaderSize, maxMIMEHeaders int64) (*Part, error) {
if r.currentPart != nil {
r.currentPart.Close()
}
@@ -357,7 +372,7 @@ func (r *Reader) nextPart(rawPart bool, maxMIMEHeaderSize int64) (*Part, error)
if r.isBoundaryDelimiterLine(line) {
r.partsRead++
- bp, err := newPart(r, rawPart, maxMIMEHeaderSize)
+ bp, err := newPart(r, rawPart, maxMIMEHeaderSize, maxMIMEHeaders)
if err != nil {
return nil, err
}
diff --git a/src/mime/multipart/readmimeheader.go b/src/mime/multipart/readmimeheader.go
index 6836928c9e8..25aa6e20928 100644
--- a/src/mime/multipart/readmimeheader.go
+++ b/src/mime/multipart/readmimeheader.go
@@ -11,4 +11,4 @@ import (
// readMIMEHeader is defined in package net/textproto.
//
//go:linkname readMIMEHeader net/textproto.readMIMEHeader
-func readMIMEHeader(r *textproto.Reader, lim int64) (textproto.MIMEHeader, error)
+func readMIMEHeader(r *textproto.Reader, maxMemory, maxHeaders int64) (textproto.MIMEHeader, error)
diff --git a/src/net/textproto/reader.go b/src/net/textproto/reader.go
index 68b445083bf..4299608d5ae 100644
--- a/src/net/textproto/reader.go
+++ b/src/net/textproto/reader.go
@@ -483,12 +483,12 @@ func (r *Reader) ReadDotLines() ([]string, error) {
// }
//
func (r *Reader) ReadMIMEHeader() (MIMEHeader, error) {
- return readMIMEHeader(r, math.MaxInt64)
+ return readMIMEHeader(r, math.MaxInt64, math.MaxInt64)
}
// readMIMEHeader is a version of ReadMIMEHeader which takes a limit on the header size.
// It is called by the mime/multipart package.
-func readMIMEHeader(r *Reader, lim int64) (MIMEHeader, error) {
+func readMIMEHeader(r *Reader, maxMemory, maxHeaders int64) (MIMEHeader, error) {
// Avoid lots of small slice allocations later by allocating one
// large one ahead of time which we'll cut up into smaller
// slices. If this isn't big enough later, we allocate small ones.
@@ -506,7 +506,7 @@ func readMIMEHeader(r *Reader, lim int64) (MIMEHeader, error) {
// Account for 400 bytes of overhead for the MIMEHeader, plus 200 bytes per entry.
// Benchmarking map creation as of go1.20, a one-entry MIMEHeader is 416 bytes and large
// MIMEHeaders average about 200 bytes per entry.
- lim -= 400
+ maxMemory -= 400
const mapEntryOverhead = 200
// The first line cannot start with a leading space.
@@ -538,6 +538,11 @@ func readMIMEHeader(r *Reader, lim int64) (MIMEHeader, error) {
continue
}
+ maxHeaders--
+ if maxHeaders < 0 {
+ return nil, errors.New("message too large")
+ }
+
// Skip initial spaces in value.
i++ // skip colon
for i < len(kv) && (kv[i] == ' ' || kv[i] == '\t') {
@@ -547,11 +552,11 @@ func readMIMEHeader(r *Reader, lim int64) (MIMEHeader, error) {
vv := m[key]
if vv == nil {
- lim -= int64(len(key))
- lim -= mapEntryOverhead
+ maxMemory -= int64(len(key))
+ maxMemory -= mapEntryOverhead
}
- lim -= int64(len(value))
- if lim < 0 {
+ maxMemory -= int64(len(value))
+ if maxMemory < 0 {
// TODO: This should be a distinguishable error (ErrMessageTooLarge)
// to allow mime/multipart to detect it.
return m, errors.New("message too large")
--
2.33.0

View File

@ -63,7 +63,7 @@
Name: golang
Version: 1.17.3
Release: 16
Release: 17
Summary: The Go Programming Language
License: BSD and Public Domain
URL: https://golang.org/
@ -183,6 +183,12 @@ Patch6030: 0030-release-branch.go1.18-net-http-update-bundled-golang.patch
Patch6031: 0031-all-update-vendored-golang.org-x-net.patch
Patch6032: 0032-crypto-tls-replace-all-usages-of-BytesOrPanic.patch
Patch6033: 0033-mime-multipart-limit-memory-inode-consumption-of-Rea.patch
Patch6034: 0034-release-branch.go1.19-net-textproto-avoid-overpredic.patch
Patch6035: 0035-release-branch.go1.19-go-scanner-reject-large-line-a.patch
Patch6036: 0036-release-branch.go1.19-html-template-disallow-actions.patch
Patch6037: 0037-release-branch.go1.19-mime-multipart-avoid-excessive.patch
Patch6038: 0038-release-branch.go1.19-net-textproto-mime-multipart-i.patch
Patch6039: 0039-release-branch.go1.19-mime-multipart-limit-parsed-mi.patch
ExclusiveArch: %{golang_arches}
@ -421,6 +427,12 @@ fi
%files devel -f go-tests.list -f go-misc.list -f go-src.list
%changelog
* Thu Apr 13 2023 hanchao <hanchao47@huawei.com> - 1.17.3-17
- Type:CVE
- CVE:CVE-2023-24534,CVE-2023-24536,CVE-2023-24537,CVE-2023-24538
- SUG:NA
- DESC: fix CVE-2023-24534,CVE-2023-24536,CVE-2023-24537,CVE-2023-24538
* Thu Apr 13 2023 penghaitao <htpengc@isoftstone.com> - 1.17.3-16
- fix bogus date in %changelog