[PATCH v2] nslookup: fix out-of-bounds reads in TXT and SOA record parsing

The TXT record handler reads the character-string length byte and
copies that many bytes without checking it against the record's
RDLENGTH. A reply with rdlen==1 and a length byte of up to 255 makes
memcpy() read past the record (and past the reply buffer); the data is
then printed, disclosing adjacent memory.

The SOA handler checks that RDLENGTH is at least 20, the size of the
five fixed 32-bit fields, but reads those fields at an offset that
depends on the two domain names preceding them. ns_name_uncompress()
bounds a name by the end of the message, not by the end of RDATA, so
the names can consume the space the check accounted for. A record with
rdlen==20 whose names are two compression pointers already reads four
bytes past RDATA; with longer names the read runs past the end of the
512-byte reply buffer. All five fields are printed.

What leaks is the stack around that buffer in send_queries(), so this
is not limited to data disclosure. In testing, a libc code pointer was
recovered at the same offset in 5 of 7 runs, and PIE text pointers turn
up as well, which gives a remote attacker an ASLR bypass rather than
just a read past the end of a record.

Clamp the TXT length to the bytes actually present in RDATA, and
recheck the remaining RDLENGTH in SOA once both names are consumed.

   text    data     bss     dec     hex filename
  20376    3936       0   24312    5ef8 networking/nslookup.o before
  20490    3936       0   24426    5f6a networking/nslookup.o after
                           +114

Signed-off-by: Ali Ahmet Memis <[email protected]>
---
v1 -> v2:
- also bound the SOA fixed fields. The existing rdlen >= 20 check does
  not survive the two variable-length names that precede them, so v1
  left that read unbounded.
- describe what the leak actually exposes.

The TXT read was also reported earlier by RZP <[email protected]>
(https://github.com/mirror/busybox/issues/122), with a fix posted to the
list by Roberto A. Foglietta. Happy to defer to that patch if it is the
one being picked up.

 networking/nslookup.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/networking/nslookup.c b/networking/nslookup.c
index b67d354f7..0dd92f443 100644
--- a/networking/nslookup.c
+++ b/networking/nslookup.c
@@ -808,6 +808,9 @@ static NOINLINE int parse_reply(const unsigned char *msg, size_t len)
 				return -1;
 			}
 			n = *(unsigned char *)ns_rr_rdata(rr);
+			/* Length byte can lie, clamp it to RDATA */
+			if (n > rdlen - 1)
+				n = rdlen - 1;
 			if (n > 0) {
 				memset(dname, 0, sizeof(dname));
 				memcpy(dname, ns_rr_rdata(rr) + 1, n);
@@ -852,6 +855,7 @@ static NOINLINE int parse_reply(const unsigned char *msg, size_t len)
 
 			printf("\torigin = %s\n", dname);
 			cp += n;
+			rdlen -= n;
 
 			n = ns_name_uncompress(ns_msg_base(handle), ns_msg_end(handle),
 			                       cp, dname, sizeof(dname));
@@ -862,6 +866,13 @@ static NOINLINE int parse_reply(const unsigned char *msg, size_t len)
 
 			printf("\tmail addr = %s\n", dname);
 			cp += n;
+			rdlen -= n;
+
+			/* Names are variable-length, recheck the fixed fields */
+			if (rdlen < 20) {
+				dbg("SOA record too short:%d\n", rdlen);
+				return -1;
+			}
 
 			printf("\tserial = %lu\n", ns_get32(cp));
 			cp += 4;
-- 
2.55.0