ithewei / ithewei/libhv

Security: OOB write/read in protocol/dns.c dns_name_decode/encode and compression path

Open
#851 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
7.6k
Forks
1.4k
Avg merge
1d 21h
Merged PRs (30d)
9

Description

## Summary

`protocol/dns.c` has multiple memory-safety bugs in DNS name encode/decode used by `dns_rr_unpack` / `dns_unpack` / `nslookup` / `dns_query`.

I am reporting these as **original findings** against current libhv `master` (lab verification 2026-07-22). I did not find a prior issue/CVE specifically for `dns_name_decode` / `dns_name_encode` bounds.

## Bug 1 — `dns_name_decode` unbounded write (CWE-120 / CWE-787)

```c
// protocol/dns.c
int dns_name_decode(const char* buf, char* domain) {
const char* p = buf;
int len = *p++;
int buflen = 1;
while (*p != '\0') {
if (len-- == 0) {
len = *p;
*domain = '.';
} else {
*domain = *p;
}
++p;
++domain; // never checks remaining space in domain[]
++buflen;
}
*domain = '\0';
...
}
```

Callers pass `rr->name` which is only `DNS_NAME_MAXLEN` (256) bytes (`dns_rr_t.name[256]`), but the decoder never enforces that limit. A crafted wire name that expands to >255 bytes overwrites past `name[]`.

There is also **no remaining-input length parameter**: a missing terminating `0x00` causes unbounded read of `buf`.

**Local confirmation**: canary after a 256-byte domain buffer is smashed (`buflen` returned 513).

## Bug 2 — compression-pointer path leaves `name[]` uninitialized (CWE-457 / CWE-125)

```c
int dns_rr_unpack(...) {
if (*(uint8_t*)p >= 192) {
namelen = 2;
// name is never written / NUL-terminated
} else {
namelen = dns_name_decode(buf, rr->name);
}
...
}
```

Any RR using a compression pointer leaves `rr->name` untouched. Later `strlen` / `%s` use is an OOB read.

## Bug 3 — `dns_name_encode` unbounded write (CWE-787)

```c
int dns_name_encode(const char* domain, char* buf) {
...
while (*p != '\0') {
...
++buf; // no capacity check
++buflen;
}
```

`dns_rr_pack` uses a fixed `char encoded_name[256]` and calls encode without checking domain length. A long `rr->name` overruns `encoded_name`.

**Local confirmation**: encode of a long name returned length 502 past a 256-byte buffer canary.

## Attack surface

Any application using libhv DNS helpers (`dns_unpack`, `dns_query`, `nslookup`, examples under protocol DNS) that processes **untrusted DNS responses** (or packs attacker-influenced names) can hit these paths.

Not the default HTTP-only build path, but enabled with protocol/DNS (`WITH_PROTOCOL` / equivalent).

## Suggested fix

1. Change APIs to pass capacity / remaining wire length, e.g.
`dns_name_decode(const char* buf, size_t buflen, char* domain, size_t domaincap)`
2. Reject expanded names `>= 256` and individual labels `> 63`.
3. On compression pointer, either fully resolve into `name[]` with bounds, or set `name[0] = '\0'` / explicit flag and never treat as C-string until resolved.
4. Cap `dns_name_encode` output to provided buffer size; return error if domain too long.
5. Add unit tests + ASan fuzz of `dns_unpack`.

## Reproduction (lab)

Standalone canary harnesses (same algorithm as tree):

- long wire name → `dns_name_decode` smashes canary after 256-byte domain
- compression pointer path → `name[]` never written
- long domain string → `dns_name_encode` smashes 256-byte encode buffer

Happy to attach minimal C files or open a PR with a patch if useful.

## Researcher

LL-V
GitHub: https://github.com/LL-V

Please credit if a CVE is requested. Testing was local-only against source / standalone reimplementation of the vulnerable functions.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in protocol/dns.c, tracing dns_name_decode and dns_name_encode through dns_rr_unpack, dns_unpack, dns_rr_pack, nslookup, and dns_query. Review the fixed DNS_NAME_MAXLEN and encoded_name buffers, then add unit tests and ASan fuzzing for dns_unpack. Done means untrusted names cannot cause out-of-bounds reads or writes, compression-pointer names are safely handled, and oversized inputs return errors.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
networking, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.