[Robustness] Crashes if QTYPE is unknown

[bortzmeyer]

There are several robustness issues in zig-dns. If a DNS server using zig-dns is exposed to the public Internet, it can crash (or worse) if the incoming packet is badly formed. Such invalid packets are common on the Internet, either by mistake or as deliberate attack.

First example, when the QTYPE is unknown, instead of returning an error (or, better, the QTYPE as just a number), zig-dns crashes:

...
 Questions {
    Question {
      Name: 
thread 192525 panic: invalid enum value
???:?:?: 0x2c6515 in format__anon_10730 (incorrect)
???:?:?: 0x2bbb9a in formatType__anon_10378 (incorrect)
???:?:?: 0x2bbb23 in format__anon_10367 (incorrect)
???:?:?: 0x2ad010 in print__anon_10104 (incorrect)
???:?:?: 0x29e807 in format__anon_9733 (incorrect)
???:?:?: 0x28c00f in formatType__anon_9306 (incorrect)
???:?:?: 0x28bf36 in format__anon_9294 (incorrect)
???:?:?: 0x276c60 in print__anon_8061 (incorrect)
???:?:?: 0x24338c in print__anon_5074 (incorrect)
???:?:?: 0x22c7db in main (incorrect)
???:?:?: 0x22bf25 in posixCallMainAndExit (incorrect)
???:?:?: 0x22ba11 in _start (incorrect)
???:?:?: 0x0 in ??? (???)
zsh: IOT instruction  ./incorrect

Here is the code to reproduce the issue:

const std = @import("std");
const io = std.io;
const dns = @import("zig-dns/src/dns.zig");

pub fn main() anyerror!void {
    var myallo = std.heap.GeneralPurposeAllocator(.{}){};
    const allocator = myallo.allocator();
    const size = 17;
    const data: [size]u8 = .{
        0x0, 0x0, // ID
        0x80, 0x0, // QR=reply, opcode, flags, rcode
        0x0, 0x1, // QDCOUNT
        0x0, 0x0, // ANCOUNT
        0x0, 0x0, // NSCOUNT
        0x0, 0x0, // ARCOUNT
        0x0,  // root domain
        0xfe, 0x1, // RR type NOT REGISTERED UNKNOWN
        0x0, 0x1, // RR class
    };
    const response = try dns.Message.from_bytes(allocator, data[0..size]);
    defer response.deinit();
    std.debug.print("Response:\n{any}\n", .{response});
}

Solving the problem is not easy in Zig. Ma be using non-exhaustive enums? https://ziggit.dev/t/catching-invalid-enum-value-errors/

[bortzmeyer]

Of course, QCLASS has probably the same problem.

[bortzmeyer]

Of course, QCLASS has probably the same problem.

For QCLASS, there is one more problem: having a closed list of classes (an exhaustive enum) will make difficult to implement later EDNS, where the "class" of the OPT resource record is the buffer size.

[bortzmeyer]

Solving the problem is not easy in Zig. Ma be using non-exhaustive enums? https://ziggit.dev/t/catching-invalid-enum-value-errors/

Actually, the type Type is already non-exhaustive. The crash is because of the use of @tagName. Using this function for a tagless value is undefined behavior. Unfortunately, it seems there is no way in Zig to catch this problem, or to check that the value has a tag.

[dantecatalfamo]

I remember running into that issue when I was initially working on it. Automatically detecting if a non-exhaustive enum's value is part of the list of defined values isn't possible currently. It's not the first project where I've run into that issue.

[dantecatalfamo]

I asked about it a while ago, it's possible to get the same effect using a check like std.enums.tagName(T, t) != null, but it's not ideal.

[bortzmeyer]

I remember running into that issue when I was initially working on it. Automatically detecting if a non-exhaustive enum's value is part of the list of defined values isn't possible currently. It's not the first project where I've run into that issue.

A solution would be to write a program that downloads and parses the XML version of the IANA registry of resource record types and generates the enum, plus a function converting from int to enum (if the int is known). Such code-generated Zig code would then be compiled.

[dantecatalfamo]

Another solution could be to add a formatter function on the Type and QType enums, and to remove @tagName completely

[dantecatalfamo]

That would probably be the easiest actually, and would prevent any other @tagName-related crashes

[dantecatalfamo]

Actually, I already have something that does that: the function formatTagName at the bottom. Sorry, it's been a while since I've touched this library.