Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insufficient validation of kemID results in panic #29

Open
fox-rose opened this issue Mar 26, 2024 · 0 comments
Open

Insufficient validation of kemID results in panic #29

fox-rose opened this issue Mar 26, 2024 · 0 comments

Comments

@fox-rose
Copy link

While auditing the unmarshal routine responsible for unmarshalling the encapsulated
request, we observed that the implementation reads the 2 byte kemID directly from the
attacker-controlled buffer without adequately checking that the attacker-controlled kemID is
valid. The implementation raises a panic() in the event that the call to kemID.Scheme()
defaults to the switch statement, resulting in a remote DoS situation.

Notably, a reference implementation utilizing the ohttp-go library wraps the entire library into
the Golang standard http library, which provides a built-in mechanism to recover from raised
panics. However, the library itself should return an appropriate error when the provided
kemID is invalid, rather than assuming a panic state.

Affected file:
ohttp-go/ohttp.go
Affected code:
func UnmarshalEncapsulatedRequest(enc []byte) (EncapsulatedRequest, error)
{
b := bytes.NewBuffer(enc)
[...]
kdfIDBuffer := make([]byte, 2)
_, err = b.Read(kdfIDBuffer)
if err != nil {
return EncapsulatedRequest{}, err
}
[...]
kemID := hpke.KEM(binary.BigEndian.Uint16(kemIDBuffer))
[...]
key := make([]byte, kemID.Scheme().CiphertextSize())
[...]
}

Affected file:
vendor/github.com/cloudflare/circl/hpke/algs.go
Affected code:
func (k KEM) Scheme() kem.AuthScheme {
switch k {
case KEM_P256_HKDF_SHA256:
return dhkemp256hkdfsha256
case KEM_P384_HKDF_SHA384:
return dhkemp384hkdfsha384
case KEM_P521_HKDF_SHA512:
return dhkemp521hkdfsha512
case KEM_X25519_HKDF_SHA256:
return dhkemx25519hkdfsha256
case KEM_X448_HKDF_SHA512:
return dhkemx448hkdfsha512
case KEM_X25519_KYBER768_DRAFT00:
return hybridkemX25519Kyber768
default:
panic(ErrInvalidKEM)
}
}
The following test function will trigger a crash by causing the KEM validation to panic:
PoC:
func TestInvalidKem(t *testing.T) {
data := []byte{0x03, 0x04, 0x50, 0x4f, 0x53, 0x54, 0x03, 0x6d, 0x30,
0x30, 0x00, 0x00,

0x00, 0x03, 0x30, 0x30, 0x30, 0x30, 0x06}
UnmarshalEncapsulatedRequest(data)
}
To mitigate this issue, we suggest utilizing the kemID.IsValid() method before invoking
kemID.Scheme().CiphertextSize(). If the kemID is invalid, false will be returned and the
UnmarshalEncapsulatedRequest function should gracefully return, rather than inducing a
panic situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant