Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix some compression bugs in dns. #4813

Merged
merged 1 commit into from
Feb 9, 2025
Merged

Conversation

haesbaert
Copy link
Contributor

@haesbaert haesbaert commented Feb 8, 2025

  • A compression pointer is when the two higher bits are set, the code was considering only 0xC0 as a pointer, where in reality anything from 0xC0-0xFF is a pointer, probably went unnoticed since you need big packets to have long pointers.
  • Make sure we can access the lower byte of the pointer by checking len, the code was careful to not access past the first byte, but ignored the second.
  • As per RFC9267 make sure a pointer only points backwards, this one is not so bad, as the code had a iteration_max that ended up guarding against infinite jumps.

Lightly tested, some eyes are welcome, but these are likely remote DOSable.

 - A compression pointer is when the two higher bits are set, the code was
   considering only 0xC0 as a pointer, where in reality anything from 0xC0-0xFF is
   a pointer, probably went unnoticed since you need big packets to have long pointers.
 - Make sure we can access the lower byte of the pointer by checking len, the
   code was careful to not access past the first byte, but ignored the second.
 - As per RFC9267 make sure a pointer only points backwards, this one is not so
   bad, as the code had a iteration_max that ended up guarding against infinite jumps.

Lightly tested, some eyes are welcome, but these are remote DOSable.
@haesbaert
Copy link
Contributor Author

I'm writing a recursive resolver in odin and eyeballed the compression code and noticed these.
Did some very light testing.

@Kelimion Kelimion merged commit fdc0115 into odin-lang:master Feb 9, 2025
7 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants