-
Notifications
You must be signed in to change notification settings - Fork 444
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
panic decoding DHCP domain name server option #305
Labels
Comments
Sounds like we should add fuzzing for DHCP packets too. |
@whitequark would you accept a PR to only use the first 3 name servers supplied by the DHCP server? This is not a long term solution, but seems useful in the short term. (This seems to be what the current code is trying to implement at the moment with its fixed array of 3 DNS servers) |
Yes, with a test for the panic. |
cjbe
added a commit
to cjbe/smoltcp
that referenced
this issue
Sep 12, 2019
The OPT_DOMAIN_NAME_SERVER DHCP option field can advertise an unbounded number of DNS servers. This adds a test that we correctly parse packets with more DNS servers advertised than we can fit in our fixed length buffer.
cjbe
added a commit
to cjbe/smoltcp
that referenced
this issue
Sep 12, 2019
The OPT_DOMAIN_NAME_SERVER option can advertise an arbitrary number of DNS servers, but we currently use a fixed length array to store these servers. This commit ensures only the first 3 (highest priority) servers are used.
bors bot
added a commit
that referenced
this issue
Nov 3, 2021
562: fuzz: DHCP header parser r=Dirbaio a=alexandrasandulescu Hi. This fuzz target tests the [DHCPRepr](https://docs.rs/smoltcp/0.7.5/smoltcp/wire/struct.DhcpRepr.html) `parse` and `emit` functions ([context](#305 (comment))). The fuzz target requires a small change in the `DHCPOption` parser because the option type `OPT_DOMAIN_NAME_SERVER ` requires the payload representing the DNS server list to have a length multiple of 4. Otherwise the `chunks` call places the remainder in the last chunk (length < 4). This panics when the last chunk is parsed into an `IPv4Address`. Do you have a better suggestion for handling this situation? I ran the fuzzer for a couple of days but it didn't find any crash yet. To increase the chances, I used the oss-fuzz seed corpora of `dnsmasq` DHCP fuzzer and `systemd` DHCP server fuzzer. Please share if you have suggestions on how to improve coverage. Co-authored-by: Alexandra Sandulescu <aesa@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Using the current smoltcp master, I get a panic during parsing a DHCP response:
This panic is from parsing the DHCP option field OPT_DOMAIN_NAME_SERVER here
The panic occurs as my DHCP server is broadcasting 4 DNS server addresses, and the code here iterates through the complete list of 4 DNS servers but tries to store the parsed addresses into an array of fixed length 3.
In my case the data in the options field is:
[163, 1, 74, 6, 163, 1, 74, 7, 163, 1, 74, 3, 163, 1, 74, 4]
This is valid, as per the RFC the only requirements are a minimum length of 4 octets, and length a multiple of 4 octets (i.e. there is no maximum length).
The text was updated successfully, but these errors were encountered: