[GH-ISSUE #2024] `proto::op::Message` can panic when `dnssec` is not enabled no debug builds #856

New issue

Closed

opened 2026-03-16 00:35:58 +03:00 by kerem · 3 comments

kerem commented

2026-03-16 00:35:58 +03:00

Owner

Originally created by @mlokr on GitHub (Sep 17, 2023).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2024

Describe the bug

On debug builds, the message decoder can panic, particularly when dnssec feature is disabled.

To Reproduce

#[test]
    fn magic_message() {
        const CRASHING_MESSAGE: &[u8] = &[
            0, 0, 132, 0, 0, 0, 0, 1, 0, 0, 0, 1, 36, 49, 101, 48, 101, 101, 51, 100, 51, 45, 100,
            52, 50, 52, 45, 52, 102, 55, 56, 45, 57, 101, 52, 99, 45, 99, 51, 56, 51, 51, 55, 55,
            56, 48, 102, 50, 98, 5, 108, 111, 99, 97, 108, 0, 0, 1, 128, 1, 0, 0, 0, 120, 0, 4,
            192, 168, 1, 17, 36, 49, 101, 48, 101, 101, 51, 100, 51, 45, 100, 52, 50, 52, 45, 52,
            102, 55, 56, 45, 57, 101, 52, 99, 45, 99, 51, 56, 51, 51, 55, 55, 56, 48, 102, 50, 98,
            5, 108, 111, 99, 97, 108, 0, 0, 47, 128, 1, 0, 0, 0, 120, 0, 5, 192, 70, 0, 1, 64,
        ];
        trust_dns_proto::op::Message::from_vec(CRASHING_MESSAGE).unwrap();
    }

Expected behavior
Receiving and error instead of panic would be nice

System:

OS: archlinux
Architecture: x86_64
Version: 6.5.3-arch1-1
rustc version: 1.74.0-nightly (84a9f4c6e 2023-08-29)

Version:
Crate: proto
Version: 23

Additional context

The error can be avoided by enabling dnssec feature, but the debug_assert! implies parsing should have failed sooner, my guess is that this function should return Unknown when this returns true because of branch here. The exact error is: record types do not match, NSEC <> Some(Unknown(47))

Originally created by @mlokr on GitHub (Sep 17, 2023). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2024 **Describe the bug** On debug builds, the message decoder can panic, particularly when `dnssec` feature is disabled. **To Reproduce** ```rs #[test] fn magic_message() { const CRASHING_MESSAGE: &[u8] = &[ 0, 0, 132, 0, 0, 0, 0, 1, 0, 0, 0, 1, 36, 49, 101, 48, 101, 101, 51, 100, 51, 45, 100, 52, 50, 52, 45, 52, 102, 55, 56, 45, 57, 101, 52, 99, 45, 99, 51, 56, 51, 51, 55, 55, 56, 48, 102, 50, 98, 5, 108, 111, 99, 97, 108, 0, 0, 1, 128, 1, 0, 0, 0, 120, 0, 4, 192, 168, 1, 17, 36, 49, 101, 48, 101, 101, 51, 100, 51, 45, 100, 52, 50, 52, 45, 52, 102, 55, 56, 45, 57, 101, 52, 99, 45, 99, 51, 56, 51, 51, 55, 55, 56, 48, 102, 50, 98, 5, 108, 111, 99, 97, 108, 0, 0, 47, 128, 1, 0, 0, 0, 120, 0, 5, 192, 70, 0, 1, 64, ]; trust_dns_proto::op::Message::from_vec(CRASHING_MESSAGE).unwrap(); } ``` **Expected behavior** Receiving and error instead of panic would be nice **System:** - OS: archlinux - Architecture: x86_64 - Version: 6.5.3-arch1-1 - rustc version: 1.74.0-nightly (84a9f4c6e 2023-08-29) **Version:** Crate: proto Version: 23 **Additional context** The error can be avoided by enabling `dnssec` feature, but the `debug_assert!` implies parsing should have failed sooner, my guess is that [this function](https://github.com/bluejekyll/trust-dns/blob/main/crates/proto/src/rr/record_type.rs#L249) should return Unknown when [this](https://github.com/bluejekyll/trust-dns/blob/main/crates/proto/src/rr/record_type.rs#L162) returns true because of [branch here](https://github.com/bluejekyll/trust-dns/blob/main/crates/proto/src/rr/record_data.rs#L855). The exact error is: `record types do not match, NSEC <> Some(Unknown(47)) `

kerem

2026-03-16 00:35:58 +03:00

closed this issue
added the
bug

crate:proto
labels

kerem commented

2026-03-16 00:36:14 +03:00

Author

Owner

@bluejekyll commented on GitHub (Sep 17, 2023):

I'm trying to reproduce this, so far no luck in #2025. Do you have the exact build and cargo parameters you used to recreate this error?

Now it is awkward that if DNSSEC is enabled or not changes if this parse is successful or not. My sense is it shouldn't be an error if DNSSEC is disabled, just should get unknown data...

@bluejekyll commented on GitHub (Sep 17, 2023): I'm trying to reproduce this, so far no luck in #2025. Do you have the exact build and cargo parameters you used to recreate this error? Now it is awkward that if DNSSEC is enabled or not changes if this parse is successful or not. My sense is it shouldn't be an error if DNSSEC is disabled, just should get unknown data...

kerem commented

2026-03-16 00:36:19 +03:00

Author

Owner

@bluejekyll commented on GitHub (Sep 18, 2023):

Ok, interestingly, making DNSClass::from_u16 infallible triggers the panic that you ran into. But it doesn't look like you did that, I kinda wonder how this happened without that change (see the second commit to #2025)

@bluejekyll commented on GitHub (Sep 18, 2023): Ok, interestingly, making `DNSClass::from_u16` infallible triggers the panic that you ran into. But it doesn't look like you did that, I kinda wonder how this happened without that change (see the second commit to #2025)

kerem commented

2026-03-16 00:36:24 +03:00

Author

Owner

@bluejekyll commented on GitHub (Sep 18, 2023):

Ok, I think by fixing a couple of constructs in the parsing code around unknown values, this might be resolved. I only found the debug panics, not any release mode panics. Please let me know if you think the fixes don't resolve this.

@bluejekyll commented on GitHub (Sep 18, 2023): Ok, I think by fixing a couple of constructs in the parsing code around unknown values, this might be resolved. I only found the `debug` panics, not any `release` mode panics. Please let me know if you think the fixes don't resolve this.

kerem referenced this issue

2026-03-16 02:23:02 +03:00

[PR #856] [CLOSED] Bump lazy_static from 1.3.0 to 1.4.0 #1742