pi-hole / FTL

The Pi-hole FTL engine
https://pi-hole.net
Other
1.34k stars 187 forks source link

Fix possible heap-corruption bug #1990

Closed DL6ER closed 3 weeks ago

DL6ER commented 4 weeks ago

What does this implement/fix?

Fix very long DNS names (>64 bytes) potentially crashing the internal name resolving mechanism, the new limit is 256 bytes with proper boundary checking. No user reports are known. This came up during my regular valgrind checking of the pihole-FTL process. Looking at the git blame for this code, the broken code has been in there for half a year seemingly without anyone noticing it. Note that this bug affects only internal name resolution used by FTL to infer host names for clients and upstream servers. It is completely separated from normal DNS operation used by the clients in your network.

Heap overflows are exploitable in a different manner to that of stack-based overflows. Intentional exploitation is unlikely given ASLR (Address Space Layout Randomization), however, not impossible. For this overflow to be abused the attacker needs to be in control of one of your specified upstream DNS server. As those are typically your router (conditional forwarding) and a large player (like Google or Cloudflare) or even another local software (like unbound), the likeliness for this is low. The overflow cannot be triggered externally, otherwise.

Even when a crash is much (!) more likely than anything else, this PR is marked with the SECURITY tag.


Related issue or feature (if applicable): N/A

Pull request in docs with documentation (if applicable): N/A


By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)

Checklist: