From bb6c6347f1f2ea4c7ee07a30dd8b7649023ed827 Mon Sep 17 00:00:00 2001 From: Julie Qiu Date: Tue, 1 Mar 2022 10:19:38 -0600 Subject: [PATCH] encoding/pem: fix stack overflow in Decode # AWS EKS Backported To: go-1.16.15-eks Backported On: Tue, 04 Oct 2022 Backported By: budris@amazon.com Backported From: release-branch.go1.17 EKS Patch Source Commit: https://github.com/danbudris/go/commit/228f3afcd8e7ee489b8a5108ac09ca59c8976bc1 Upstream Source Commit: https://github.com/golang/go/commit/2116d60993e90d3f9b963c979f4bf1d116af03ff # Original Information Previously, Decode called decodeError, a recursive function that was prone to stack overflows when given a large PEM file containing errors. Credit to Juho Nurminen of Mattermost who reported the error. Fixes CVE-2022-24675 Updates #51853 Fixes #52036 Change-Id: Iffe768be53c8ddc0036fea0671d290f8f797692c Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1391157 Reviewed-by: Damien Neil Reviewed-by: Filippo Valsorda (cherry picked from commit 794ea5e828010e8b68493b2fc6d2963263195a02) Reviewed-on: https://go-review.googlesource.com/c/go/+/399816 Run-TryBot: Dmitri Shuralyov Reviewed-by: Dmitri Shuralyov Reviewed-by: Cherry Mui TryBot-Result: Gopher Robot --- src/encoding/pem/pem.go | 174 +++++++++++++++-------------------- src/encoding/pem/pem_test.go | 28 +++++- 2 files changed, 101 insertions(+), 101 deletions(-) diff --git a/src/encoding/pem/pem.go b/src/encoding/pem/pem.go index a7272da5ad3a8..1bee1c12d286f 100644 --- a/src/encoding/pem/pem.go +++ b/src/encoding/pem/pem.go @@ -87,123 +87,97 @@ func Decode(data []byte) (p *Block, rest []byte) { // pemStart begins with a newline. However, at the very beginning of // the byte array, we'll accept the start string without it. rest = data - if bytes.HasPrefix(data, pemStart[1:]) { - rest = rest[len(pemStart)-1 : len(data)] - } else if i := bytes.Index(data, pemStart); i >= 0 { - rest = rest[i+len(pemStart) : len(data)] - } else { - return nil, data - } - - typeLine, rest := getLine(rest) - if !bytes.HasSuffix(typeLine, pemEndOfLine) { - return decodeError(data, rest) - } - typeLine = typeLine[0 : len(typeLine)-len(pemEndOfLine)] - - p = &Block{ - Headers: make(map[string]string), - Type: string(typeLine), - } - for { - // This loop terminates because getLine's second result is - // always smaller than its argument. - if len(rest) == 0 { + if bytes.HasPrefix(rest, pemStart[1:]) { + rest = rest[len(pemStart)-1:] + } else if i := bytes.Index(rest, pemStart); i >= 0 { + rest = rest[i+len(pemStart) : len(rest)] + } else { return nil, data } - line, next := getLine(rest) - i := bytes.IndexByte(line, ':') - if i == -1 { - break + var typeLine []byte + typeLine, rest = getLine(rest) + if !bytes.HasSuffix(typeLine, pemEndOfLine) { + continue } + typeLine = typeLine[0 : len(typeLine)-len(pemEndOfLine)] - // TODO(agl): need to cope with values that spread across lines. - key, val := line[:i], line[i+1:] - key = bytes.TrimSpace(key) - val = bytes.TrimSpace(val) - p.Headers[string(key)] = string(val) - rest = next - } + p = &Block{ + Headers: make(map[string]string), + Type: string(typeLine), + } - var endIndex, endTrailerIndex int + for { + // This loop terminates because getLine's second result is + // always smaller than its argument. + if len(rest) == 0 { + return nil, data + } + line, next := getLine(rest) - // If there were no headers, the END line might occur - // immediately, without a leading newline. - if len(p.Headers) == 0 && bytes.HasPrefix(rest, pemEnd[1:]) { - endIndex = 0 - endTrailerIndex = len(pemEnd) - 1 - } else { - endIndex = bytes.Index(rest, pemEnd) - endTrailerIndex = endIndex + len(pemEnd) - } + i := bytes.IndexByte(line, ':') + if i == -1 { + break + } - if endIndex < 0 { - return decodeError(data, rest) - } + // TODO(agl): need to cope with values that spread across lines. + key, val := line[:i], line[i+1:] + key = bytes.TrimSpace(key) + val = bytes.TrimSpace(val) + p.Headers[string(key)] = string(val) + rest = next + } - // After the "-----" of the ending line, there should be the same type - // and then a final five dashes. - endTrailer := rest[endTrailerIndex:] - endTrailerLen := len(typeLine) + len(pemEndOfLine) - if len(endTrailer) < endTrailerLen { - return decodeError(data, rest) - } + var endIndex, endTrailerIndex int - restOfEndLine := endTrailer[endTrailerLen:] - endTrailer = endTrailer[:endTrailerLen] - if !bytes.HasPrefix(endTrailer, typeLine) || - !bytes.HasSuffix(endTrailer, pemEndOfLine) { - return decodeError(data, rest) - } + // If there were no headers, the END line might occur + // immediately, without a leading newline. + if len(p.Headers) == 0 && bytes.HasPrefix(rest, pemEnd[1:]) { + endIndex = 0 + endTrailerIndex = len(pemEnd) - 1 + } else { + endIndex = bytes.Index(rest, pemEnd) + endTrailerIndex = endIndex + len(pemEnd) + } - // The line must end with only whitespace. - if s, _ := getLine(restOfEndLine); len(s) != 0 { - return decodeError(data, rest) - } + if endIndex < 0 { + continue + } - base64Data := removeSpacesAndTabs(rest[:endIndex]) - p.Bytes = make([]byte, base64.StdEncoding.DecodedLen(len(base64Data))) - n, err := base64.StdEncoding.Decode(p.Bytes, base64Data) - if err != nil { - return decodeError(data, rest) - } - p.Bytes = p.Bytes[:n] + // After the "-----" of the ending line, there should be the same type + // and then a final five dashes. + endTrailer := rest[endTrailerIndex:] + endTrailerLen := len(typeLine) + len(pemEndOfLine) + if len(endTrailer) < endTrailerLen { + continue + } + + restOfEndLine := endTrailer[endTrailerLen:] + endTrailer = endTrailer[:endTrailerLen] + if !bytes.HasPrefix(endTrailer, typeLine) || + !bytes.HasSuffix(endTrailer, pemEndOfLine) { + continue + } - // the -1 is because we might have only matched pemEnd without the - // leading newline if the PEM block was empty. - _, rest = getLine(rest[endIndex+len(pemEnd)-1:]) + // The line must end with only whitespace. + if s, _ := getLine(restOfEndLine); len(s) != 0 { + continue + } - return -} + base64Data := removeSpacesAndTabs(rest[:endIndex]) + p.Bytes = make([]byte, base64.StdEncoding.DecodedLen(len(base64Data))) + n, err := base64.StdEncoding.Decode(p.Bytes, base64Data) + if err != nil { + continue + } + p.Bytes = p.Bytes[:n] -func decodeError(data, rest []byte) (*Block, []byte) { - // If we get here then we have rejected a likely looking, but - // ultimately invalid PEM block. We need to start over from a new - // position. We have consumed the preamble line and will have consumed - // any lines which could be header lines. However, a valid preamble - // line is not a valid header line, therefore we cannot have consumed - // the preamble line for the any subsequent block. Thus, we will always - // find any valid block, no matter what bytes precede it. - // - // For example, if the input is - // - // -----BEGIN MALFORMED BLOCK----- - // junk that may look like header lines - // or data lines, but no END line - // - // -----BEGIN ACTUAL BLOCK----- - // realdata - // -----END ACTUAL BLOCK----- - // - // we've failed to parse using the first BEGIN line - // and now will try again, using the second BEGIN line. - p, rest := Decode(rest) - if p == nil { - rest = data + // the -1 is because we might have only matched pemEnd without the + // leading newline if the PEM block was empty. + _, rest = getLine(rest[endIndex+len(pemEnd)-1:]) + return p, rest } - return p, rest } const pemLineLength = 64 diff --git a/src/encoding/pem/pem_test.go b/src/encoding/pem/pem_test.go index b2b6b15e73655..c94b5ca53b568 100644 --- a/src/encoding/pem/pem_test.go +++ b/src/encoding/pem/pem_test.go @@ -107,6 +107,12 @@ const pemMissingEndingSpace = ` dGVzdA== -----ENDBAR-----` +const pemMissingEndLine = ` +-----BEGIN FOO----- +Header: 1` + +var pemRepeatingBegin = strings.Repeat("-----BEGIN \n", 10) + var badPEMTests = []struct { name string input string @@ -131,14 +137,34 @@ var badPEMTests = []struct { "missing ending space", pemMissingEndingSpace, }, + { + "repeating begin", + pemRepeatingBegin, + }, + { + "missing end line", + pemMissingEndLine, + }, } func TestBadDecode(t *testing.T) { for _, test := range badPEMTests { - result, _ := Decode([]byte(test.input)) + result, rest := Decode([]byte(test.input)) if result != nil { t.Errorf("unexpected success while parsing %q", test.name) } + if string(rest) != test.input { + t.Errorf("unexpected rest: %q; want = %q", rest, test.input) + } + } +} + +func TestCVE202224675(t *testing.T) { + // Prior to CVE-2022-24675, this input would cause a stack overflow. + input := []byte(strings.Repeat("-----BEGIN \n", 10000000)) + result, rest := Decode(input) + if result != nil || !reflect.DeepEqual(rest, input) { + t.Errorf("Encode of %#v decoded as %#v", input, rest) } }