Skip to content

Commit 3a72103

Browse files
committed
Fix panic unmarshaling ECDHE_PSK ServerKeyExchange
Missing bounds check a invalid ServerKeyExchange message could cause crash.
1 parent e4b257d commit 3a72103

2 files changed

Lines changed: 119 additions & 0 deletions

File tree

pkg/protocol/handshake/message_server_key_exchange.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ func (m *MessageServerKeyExchange) Unmarshal(data []byte) error { //nolint:cyclo
9696
return errLengthMismatch
9797
}
9898

99+
if len(data) == 0 {
100+
return errBufferTooSmall
101+
}
102+
99103
if _, ok := elliptic.CurveTypes()[elliptic.CurveType(data[0])]; ok {
100104
m.EllipticCurveType = elliptic.CurveType(data[0])
101105
} else {

pkg/protocol/handshake/message_server_key_exchange_test.go

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,118 @@ func TestHandshakeMessageServerKeyExchange(t *testing.T) {
7171
test(rawServerKeyExchange, parsedServerKeyExchange)
7272
})
7373
}
74+
75+
func TestHandshakeMessageServerKeyExchangeUnmarshalErrors(t *testing.T) {
76+
for _, test := range []struct {
77+
name string
78+
keyExchangeAlgorithm types.KeyExchangeAlgorithm
79+
data []byte
80+
expectedErr error
81+
}{
82+
{
83+
name: "BufferTooSmall",
84+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
85+
data: []byte{0x00},
86+
expectedErr: errBufferTooSmall,
87+
},
88+
{
89+
name: "CipherSuiteUnset",
90+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmNone,
91+
data: []byte{0x00, 0x00},
92+
expectedErr: errCipherSuiteUnset,
93+
},
94+
{
95+
// PSK-only: a non-empty body remains after the identity hint.
96+
name: "PskLengthMismatch",
97+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmPsk,
98+
data: []byte{0x00, 0x00, 0xAA},
99+
expectedErr: errLengthMismatch,
100+
},
101+
{
102+
// An algorithm that is neither PSK nor ECDHE is unsupported here.
103+
name: "UnsupportedKeyExchangeAlgorithm",
104+
keyExchangeAlgorithm: types.KeyExchangeAlgorithm(1),
105+
data: []byte{0x00, 0x00},
106+
expectedErr: errLengthMismatch,
107+
},
108+
{
109+
// ECDHE_PSK where the (empty) identity hint consumes the whole body,
110+
// leaving nothing for the ECDHE parameters. Previously panicked.
111+
name: "EcdhePskEmptyHintConsumesBody",
112+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmPsk | types.KeyExchangeAlgorithmEcdhe,
113+
data: []byte{0x00, 0x00},
114+
expectedErr: errBufferTooSmall,
115+
},
116+
{
117+
name: "InvalidEllipticCurveType",
118+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
119+
data: []byte{0x99, 0x00},
120+
expectedErr: errInvalidEllipticCurveType,
121+
},
122+
{
123+
// Valid curve type but not enough bytes for the named curve.
124+
name: "NamedCurveBufferTooSmall",
125+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
126+
data: []byte{0x03, 0x00},
127+
expectedErr: errBufferTooSmall,
128+
},
129+
{
130+
name: "InvalidNamedCurve",
131+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
132+
data: []byte{0x03, 0xFF, 0xFF},
133+
expectedErr: errInvalidNamedCurve,
134+
},
135+
{
136+
// Valid curve type and named curve but missing the public key length.
137+
name: "PublicKeyLengthBufferTooSmall",
138+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
139+
data: []byte{0x03, 0x00, 0x1d},
140+
expectedErr: errBufferTooSmall,
141+
},
142+
{
143+
// Public key length exceeds the remaining body.
144+
name: "PublicKeyBufferTooSmall",
145+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
146+
data: []byte{0x03, 0x00, 0x1d, 0x05},
147+
expectedErr: errBufferTooSmall,
148+
},
149+
{
150+
name: "InvalidHashAlgorithm",
151+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
152+
data: []byte{0x03, 0x00, 0x1d, 0x00, 0x07},
153+
expectedErr: errInvalidHashAlgorithm,
154+
},
155+
{
156+
// Valid hash algorithm but missing the signature algorithm byte.
157+
name: "SignatureAlgorithmBufferTooSmall",
158+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
159+
data: []byte{0x03, 0x00, 0x1d, 0x00, 0x04},
160+
expectedErr: errBufferTooSmall,
161+
},
162+
{
163+
name: "InvalidSignatureAlgorithm",
164+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
165+
data: []byte{0x03, 0x00, 0x1d, 0x00, 0x04, 0x02},
166+
expectedErr: errInvalidSignatureAlgorithm,
167+
},
168+
{
169+
// Valid hash and signature algorithms but missing the signature length.
170+
name: "SignatureLengthBufferTooSmall",
171+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
172+
data: []byte{0x03, 0x00, 0x1d, 0x00, 0x04, 0x03},
173+
expectedErr: errBufferTooSmall,
174+
},
175+
{
176+
// Signature length exceeds the remaining body.
177+
name: "SignatureBufferTooSmall",
178+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
179+
data: []byte{0x03, 0x00, 0x1d, 0x00, 0x04, 0x03, 0x00, 0x05},
180+
expectedErr: errBufferTooSmall,
181+
},
182+
} {
183+
c := &MessageServerKeyExchange{
184+
KeyExchangeAlgorithm: test.keyExchangeAlgorithm,
185+
}
186+
assert.ErrorIs(t, c.Unmarshal(test.data), test.expectedErr, test.name)
187+
}
188+
}

0 commit comments

Comments
 (0)