Skip to content

Commit 2116d60

Browse files
julieqiudmitshur
julieqiu
authored and
dmitshur
committed
[release-branch.go1.17] encoding/pem: fix stack overflow in Decode
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 <[email protected]> Reviewed-by: Filippo Valsorda <[email protected]> (cherry picked from commit 794ea5e828010e8b68493b2fc6d2963263195a02) Reviewed-on: https://go-review.googlesource.com/c/go/+/399816 Run-TryBot: Dmitri Shuralyov <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: Cherry Mui <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent 7139e8b commit 2116d60

File tree

2 files changed

+101
-101
lines changed

2 files changed

+101
-101
lines changed

‎src/encoding/pem/pem.go

+74-100
Original file line numberDiff line numberDiff line change
@@ -87,123 +87,97 @@ func Decode(data []byte) (p *Block, rest []byte) {
8787
// pemStart begins with a newline. However, at the very beginning of
8888
// the byte array, we'll accept the start string without it.
8989
rest = data
90-
if bytes.HasPrefix(data, pemStart[1:]) {
91-
rest = rest[len(pemStart)-1 : len(data)]
92-
} else if i := bytes.Index(data, pemStart); i >= 0 {
93-
rest = rest[i+len(pemStart) : len(data)]
94-
} else {
95-
return nil, data
96-
}
97-
98-
typeLine, rest := getLine(rest)
99-
if !bytes.HasSuffix(typeLine, pemEndOfLine) {
100-
return decodeError(data, rest)
101-
}
102-
typeLine = typeLine[0 : len(typeLine)-len(pemEndOfLine)]
103-
104-
p = &Block{
105-
Headers: make(map[string]string),
106-
Type: string(typeLine),
107-
}
108-
10990
for {
110-
// This loop terminates because getLine's second result is
111-
// always smaller than its argument.
112-
if len(rest) == 0 {
91+
if bytes.HasPrefix(rest, pemStart[1:]) {
92+
rest = rest[len(pemStart)-1:]
93+
} else if i := bytes.Index(rest, pemStart); i >= 0 {
94+
rest = rest[i+len(pemStart) : len(rest)]
95+
} else {
11396
return nil, data
11497
}
115-
line, next := getLine(rest)
11698

117-
i := bytes.IndexByte(line, ':')
118-
if i == -1 {
119-
break
99+
var typeLine []byte
100+
typeLine, rest = getLine(rest)
101+
if !bytes.HasSuffix(typeLine, pemEndOfLine) {
102+
continue
120103
}
104+
typeLine = typeLine[0 : len(typeLine)-len(pemEndOfLine)]
121105

122-
// TODO(agl): need to cope with values that spread across lines.
123-
key, val := line[:i], line[i+1:]
124-
key = bytes.TrimSpace(key)
125-
val = bytes.TrimSpace(val)
126-
p.Headers[string(key)] = string(val)
127-
rest = next
128-
}
106+
p = &Block{
107+
Headers: make(map[string]string),
108+
Type: string(typeLine),
109+
}
129110

130-
var endIndex, endTrailerIndex int
111+
for {
112+
// This loop terminates because getLine's second result is
113+
// always smaller than its argument.
114+
if len(rest) == 0 {
115+
return nil, data
116+
}
117+
line, next := getLine(rest)
131118

132-
// If there were no headers, the END line might occur
133-
// immediately, without a leading newline.
134-
if len(p.Headers) == 0 && bytes.HasPrefix(rest, pemEnd[1:]) {
135-
endIndex = 0
136-
endTrailerIndex = len(pemEnd) - 1
137-
} else {
138-
endIndex = bytes.Index(rest, pemEnd)
139-
endTrailerIndex = endIndex + len(pemEnd)
140-
}
119+
i := bytes.IndexByte(line, ':')
120+
if i == -1 {
121+
break
122+
}
141123

142-
if endIndex < 0 {
143-
return decodeError(data, rest)
144-
}
124+
// TODO(agl): need to cope with values that spread across lines.
125+
key, val := line[:i], line[i+1:]
126+
key = bytes.TrimSpace(key)
127+
val = bytes.TrimSpace(val)
128+
p.Headers[string(key)] = string(val)
129+
rest = next
130+
}
145131

146-
// After the "-----" of the ending line, there should be the same type
147-
// and then a final five dashes.
148-
endTrailer := rest[endTrailerIndex:]
149-
endTrailerLen := len(typeLine) + len(pemEndOfLine)
150-
if len(endTrailer) < endTrailerLen {
151-
return decodeError(data, rest)
152-
}
132+
var endIndex, endTrailerIndex int
153133

154-
restOfEndLine := endTrailer[endTrailerLen:]
155-
endTrailer = endTrailer[:endTrailerLen]
156-
if !bytes.HasPrefix(endTrailer, typeLine) ||
157-
!bytes.HasSuffix(endTrailer, pemEndOfLine) {
158-
return decodeError(data, rest)
159-
}
134+
// If there were no headers, the END line might occur
135+
// immediately, without a leading newline.
136+
if len(p.Headers) == 0 && bytes.HasPrefix(rest, pemEnd[1:]) {
137+
endIndex = 0
138+
endTrailerIndex = len(pemEnd) - 1
139+
} else {
140+
endIndex = bytes.Index(rest, pemEnd)
141+
endTrailerIndex = endIndex + len(pemEnd)
142+
}
160143

161-
// The line must end with only whitespace.
162-
if s, _ := getLine(restOfEndLine); len(s) != 0 {
163-
return decodeError(data, rest)
164-
}
144+
if endIndex < 0 {
145+
continue
146+
}
165147

166-
base64Data := removeSpacesAndTabs(rest[:endIndex])
167-
p.Bytes = make([]byte, base64.StdEncoding.DecodedLen(len(base64Data)))
168-
n, err := base64.StdEncoding.Decode(p.Bytes, base64Data)
169-
if err != nil {
170-
return decodeError(data, rest)
171-
}
172-
p.Bytes = p.Bytes[:n]
148+
// After the "-----" of the ending line, there should be the same type
149+
// and then a final five dashes.
150+
endTrailer := rest[endTrailerIndex:]
151+
endTrailerLen := len(typeLine) + len(pemEndOfLine)
152+
if len(endTrailer) < endTrailerLen {
153+
continue
154+
}
155+
156+
restOfEndLine := endTrailer[endTrailerLen:]
157+
endTrailer = endTrailer[:endTrailerLen]
158+
if !bytes.HasPrefix(endTrailer, typeLine) ||
159+
!bytes.HasSuffix(endTrailer, pemEndOfLine) {
160+
continue
161+
}
173162

174-
// the -1 is because we might have only matched pemEnd without the
175-
// leading newline if the PEM block was empty.
176-
_, rest = getLine(rest[endIndex+len(pemEnd)-1:])
163+
// The line must end with only whitespace.
164+
if s, _ := getLine(restOfEndLine); len(s) != 0 {
165+
continue
166+
}
177167

178-
return
179-
}
168+
base64Data := removeSpacesAndTabs(rest[:endIndex])
169+
p.Bytes = make([]byte, base64.StdEncoding.DecodedLen(len(base64Data)))
170+
n, err := base64.StdEncoding.Decode(p.Bytes, base64Data)
171+
if err != nil {
172+
continue
173+
}
174+
p.Bytes = p.Bytes[:n]
180175

181-
func decodeError(data, rest []byte) (*Block, []byte) {
182-
// If we get here then we have rejected a likely looking, but
183-
// ultimately invalid PEM block. We need to start over from a new
184-
// position. We have consumed the preamble line and will have consumed
185-
// any lines which could be header lines. However, a valid preamble
186-
// line is not a valid header line, therefore we cannot have consumed
187-
// the preamble line for the any subsequent block. Thus, we will always
188-
// find any valid block, no matter what bytes precede it.
189-
//
190-
// For example, if the input is
191-
//
192-
// -----BEGIN MALFORMED BLOCK-----
193-
// junk that may look like header lines
194-
// or data lines, but no END line
195-
//
196-
// -----BEGIN ACTUAL BLOCK-----
197-
// realdata
198-
// -----END ACTUAL BLOCK-----
199-
//
200-
// we've failed to parse using the first BEGIN line
201-
// and now will try again, using the second BEGIN line.
202-
p, rest := Decode(rest)
203-
if p == nil {
204-
rest = data
176+
// the -1 is because we might have only matched pemEnd without the
177+
// leading newline if the PEM block was empty.
178+
_, rest = getLine(rest[endIndex+len(pemEnd)-1:])
179+
return p, rest
205180
}
206-
return p, rest
207181
}
208182

209183
const pemLineLength = 64

‎src/encoding/pem/pem_test.go

+27-1
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,12 @@ const pemMissingEndingSpace = `
107107
dGVzdA==
108108
-----ENDBAR-----`
109109

110+
const pemMissingEndLine = `
111+
-----BEGIN FOO-----
112+
Header: 1`
113+
114+
var pemRepeatingBegin = strings.Repeat("-----BEGIN \n", 10)
115+
110116
var badPEMTests = []struct {
111117
name string
112118
input string
@@ -131,14 +137,34 @@ var badPEMTests = []struct {
131137
"missing ending space",
132138
pemMissingEndingSpace,
133139
},
140+
{
141+
"repeating begin",
142+
pemRepeatingBegin,
143+
},
144+
{
145+
"missing end line",
146+
pemMissingEndLine,
147+
},
134148
}
135149

136150
func TestBadDecode(t *testing.T) {
137151
for _, test := range badPEMTests {
138-
result, _ := Decode([]byte(test.input))
152+
result, rest := Decode([]byte(test.input))
139153
if result != nil {
140154
t.Errorf("unexpected success while parsing %q", test.name)
141155
}
156+
if string(rest) != test.input {
157+
t.Errorf("unexpected rest: %q; want = %q", rest, test.input)
158+
}
159+
}
160+
}
161+
162+
func TestCVE202224675(t *testing.T) {
163+
// Prior to CVE-2022-24675, this input would cause a stack overflow.
164+
input := []byte(strings.Repeat("-----BEGIN \n", 10000000))
165+
result, rest := Decode(input)
166+
if result != nil || !reflect.DeepEqual(rest, input) {
167+
t.Errorf("Encode of %#v decoded as %#v", input, rest)
142168
}
143169
}
144170

0 commit comments

Comments
 (0)