Skip to content

Commit 49458d6

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

2 files changed

Lines changed: 121 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: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,120 @@ 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+
// Valid signature algorithm (0x03 = ECDSA) but invalid hash algorithm (0x07).
151+
name: "InvalidHashAlgorithm",
152+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
153+
data: []byte{0x03, 0x00, 0x1d, 0x00, 0x07, 0x03},
154+
expectedErr: errInvalidSignHashAlgorithm,
155+
},
156+
{
157+
// Valid hash algorithm but missing the signature algorithm byte.
158+
name: "SignatureAlgorithmBufferTooSmall",
159+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
160+
data: []byte{0x03, 0x00, 0x1d, 0x00, 0x04},
161+
expectedErr: errBufferTooSmall,
162+
},
163+
{
164+
// Valid hash algorithm (0x04 = SHA256) but invalid signature algorithm (0x02).
165+
name: "InvalidSignatureAlgorithm",
166+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
167+
data: []byte{0x03, 0x00, 0x1d, 0x00, 0x04, 0x02},
168+
expectedErr: errInvalidSignHashAlgorithm,
169+
},
170+
{
171+
// Valid hash and signature algorithms but missing the signature length.
172+
name: "SignatureLengthBufferTooSmall",
173+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
174+
data: []byte{0x03, 0x00, 0x1d, 0x00, 0x04, 0x03},
175+
expectedErr: errBufferTooSmall,
176+
},
177+
{
178+
// Signature length exceeds the remaining body.
179+
name: "SignatureBufferTooSmall",
180+
keyExchangeAlgorithm: types.KeyExchangeAlgorithmEcdhe,
181+
data: []byte{0x03, 0x00, 0x1d, 0x00, 0x04, 0x03, 0x00, 0x05},
182+
expectedErr: errBufferTooSmall,
183+
},
184+
} {
185+
c := &MessageServerKeyExchange{
186+
KeyExchangeAlgorithm: test.keyExchangeAlgorithm,
187+
}
188+
assert.ErrorIs(t, c.Unmarshal(test.data), test.expectedErr, test.name)
189+
}
190+
}

0 commit comments

Comments
 (0)