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

Malformed Router Advertisment packet in DHCPv6 (with patch) #5840

Open
3 tasks done
yamanq opened this issue May 22, 2023 · 4 comments · May be fixed by #5841
Open
3 tasks done

Malformed Router Advertisment packet in DHCPv6 (with patch) #5840

yamanq opened this issue May 22, 2023 · 4 comments · May be fixed by #5841
Labels
needs investigation Needs to be reproduced reliably.

Comments

@yamanq
Copy link

yamanq commented May 22, 2023

Prerequisites

  • I have checked the Wiki and Discussions and found no answer

  • I have searched other issues and found no duplicates

  • I want to report a bug and not ask a question

Operating system type

Linux, Other (please mention the version in the description)

CPU architecture

AMD64

Installation

GitHub releases or script from README

Setup

On one machine

AdGuard Home version

v0.107.29

Description

What did you do?

I am using AdGuard for a couple DNS rewrites and as a DHCP server in order to provide AdGuard as the DNS server (my router has no options to override the DNS server). For IPv4, this worked perfectly, and my server was set as the DNSv4 nameserver. For IPv6, the router was still used as the DNS server. I used wireshark to analyze the packets and discovered that the packets are malformed due to the MAC address being padded by zeros (See images)

Original packet:

Router Advertisement packet from Adguard. Note that the MAC address is suffixed by 2 empty bytes before the next ICMPv6 option (which is the DNS option). (MAC address has been replaced with XX)

0070   xx xx xx xx xx xx 00 00 19 03 00 00 00 00 0e 10
0080   fe 80 00 00 00 00 00 00 cc 88 e2 8d 78 03 44 3f

The packet after the Source Link-layer Address packet is then malformed since it begins with zeros.
image

Edited packet:

If I edit that packet to remove those zeros, the packet is detected properly:

0070   xx xx xx xx xx xx 19 03 00 00 00 00 0e 10 fe 80
0080   00 00 00 00 00 00 cc 88 e2 8d 78 03 44 3f

image

Additional information

I believe that the cause of this is hwAddrToLinkLayerAddr which converts MAC addresses to 8 bytes. While the RFC says that the length of the packet must be 8 bytes, I believe that is: Option Type (1 byte) + Length (1 byte) + address (6 bytes) = 8 bytes. Based on my inspection of Router Advertisement packets of my router, the address is 6 bytes. The RFC linked at the start of the function matches this interpretation:

Length         The length of the option (including the type and
                     length fields) in units of 8 octets.

I have written a pull request that fixes this issue: #5841

@yamanq yamanq linked a pull request May 22, 2023 that will close this issue
@ghost
Copy link

ghost commented Apr 2, 2024

Hi @yamanq, are you still experiencing this in the latest version?

@ghost ghost added the waiting for data Waiting for users to provide more data. label Apr 2, 2024
@yamanq
Copy link
Author

yamanq commented Apr 2, 2024

@jslawler-gh The linked PR hasn't been merged and I don't see any changes in the relevant code on the master branch. Is there a reason why you suspect that the bug has been resolved?

@ghost
Copy link

ghost commented Apr 2, 2024

It's been 11 months since this was raised and many versions have been released since then which may have impacted that result in one way or another from other pr's.

I'm confirming that you're still able to reproduce this issue or whether it has changed in some way, or is no longer there.

@yamanq
Copy link
Author

yamanq commented Apr 2, 2024

I no longer have AdGuard installed but I am confident that the bug still exists given that the code has not changed. The PR to fix this bug has been waiting for approval for about 11 months as well.

@ghost ghost added needs investigation Needs to be reproduced reliably. and removed waiting for data Waiting for users to provide more data. labels Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs investigation Needs to be reproduced reliably.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant