mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Subject: Merging ns_parse from Alpine
Date: Sat, 13 Dec 2014 19:43:20 -0500	[thread overview]
Message-ID: <20141214004320.GA17102@brightrain.aerifal.cx> (raw)

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

I'm working on merging Timo's patch for ns_parse:

http://git.alpinelinux.org/cgit/aports/tree/main/musl/1001-add-basic-dns-record-parsing-functions.patch?id=81d50064c335467fdfd80368bac6707d70db1af7

The first issue that came up in the process is that arpa/nameser.h,
which was previously not used by musl itself and really should never
have been accepted in its current form, is full of junk like
statement-expressions. Including it in a file that will be compiled
with musl adds build dependency on these nonstandard features. I
cleaned that up with no problem (just un-inlining the macros since
we're adding function versions anyway), but there are a few more
issues.

The main issue is that the parser functions have pointer arithmetic
overflows (UB) checking against the end-of-message pointer. I've tried
to fix that and I'm attaching a patch for review, along with my
version of the fixed file. I'd appreciate comments on whether I missed
anything.

Other changes were mostly cosmetic or at least mechanical.

Rich

[-- Attachment #2: ns_parse_changes_v1.diff --]
[-- Type: text/plain, Size: 3504 bytes --]

--- src/network/ns_parse.c.orig
+++ src/network/ns_parse.c
@@ -23,28 +23,28 @@
 	{ 0x0000, 0 },
 };
 
-u_int ns_get16(const unsigned char *cp)
+unsigned ns_get16(const unsigned char *cp)
 {
-	u_short s;
-	NS_GET16(s, cp);
-	return s;
+	return cp[0]<<8 | cp[1];
 }
 
-u_long ns_get32(const unsigned char *cp)
+unsigned long ns_get32(const unsigned char *cp)
 {
-	u_long l;
-	NS_GET32(l, cp);
-	return l;
+	return (unsigned)cp[0]<<24 | cp[1]<<16 | cp[2]<<8 | cp[3];
 }
 
-void ns_put16(u_int s, unsigned char *cp)
+void ns_put16(unsigned s, unsigned char *cp)
 {
-	NS_PUT16(s, cp);
+	*cp++ = s>>8;
+	*cp++ = s;
 }
 
-void ns_put32(u_long l, unsigned char *cp)
+void ns_put32(unsigned long l, unsigned char *cp)
 {
-	NS_PUT32(l, cp);
+	*cp++ = l>>24;
+	*cp++ = l>>16;
+	*cp++ = l>>8;
+	*cp++ = l;
 }
 
 int ns_initparse(const unsigned char *msg, int msglen, ns_msg *handle)
@@ -56,8 +56,11 @@
 	if (msglen < (2 + ns_s_max) * NS_INT16SZ) goto bad;
 	NS_GET16(handle->_id, msg);
 	NS_GET16(handle->_flags, msg);
-	for (i = 0; i < ns_s_max; i++) NS_GET16(handle->_counts[i], msg);
 	for (i = 0; i < ns_s_max; i++) {
+		if (NS_INT16SZ > handle->_eom - msg) goto bad;
+		NS_GET16(handle->_counts[i], msg);
+	}
+	for (i = 0; i < ns_s_max; i++) {
 		if (handle->_counts[i]) {
 			handle->_sections[i] = msg;
 			r = ns_skiprr(msg, handle->_eom, i, handle->_counts[i]);
@@ -77,23 +80,24 @@
 	return -1;
 }
 
-int ns_skiprr(const u_char *ptr, const u_char *eom, ns_sect section, int count)
+int ns_skiprr(const unsigned char *ptr, const unsigned char *eom, ns_sect section, int count)
 {
-	const u_char *p = ptr;
+	const unsigned char *p = ptr;
 	int r;
 
 	while (count--) {
 		r = dn_skipname(p, eom);
 		if (r < 0) goto bad;
+		if (r + 2 * NS_INT16SZ > eom - p) goto bad;
 		p += r + 2 * NS_INT16SZ;
 		if (section != ns_s_qd) {
-			if (p + NS_INT32SZ + NS_INT16SZ > eom) goto bad;
+			if (NS_INT32SZ + NS_INT16SZ > eom - p) goto bad;
 			p += NS_INT32SZ;
 			NS_GET16(r, p);
+			if (r > eom - p) goto bad;
 			p += r;
 		}
 	}
-	if (p > eom) goto bad;
 	return ptr - p;
 bad:
 	errno = EMSGSIZE;
@@ -125,14 +129,14 @@
 	r = dn_expand(handle->_msg, handle->_eom, handle->_msg_ptr, rr->name, NS_MAXDNAME);
 	if (r < 0) return -1;
 	handle->_msg_ptr += r;
-	if (handle->_msg_ptr + 2 * NS_INT16SZ > handle->_eom) goto size;
+	if (2 * NS_INT16SZ > handle->_eom - handle->_msg_ptr) goto size;
 	NS_GET16(rr->type, handle->_msg_ptr);
 	NS_GET16(rr->rr_class, handle->_msg_ptr);
 	if (section != ns_s_qd) {
-		if (handle->_msg_ptr + NS_INT32SZ + NS_INT16SZ > handle->_eom) goto size;
+		if (NS_INT32SZ + NS_INT16SZ > handle->_eom - handle->_msg_ptr) goto size;
 		NS_GET32(rr->ttl, handle->_msg_ptr);
 		NS_GET16(rr->rdlength, handle->_msg_ptr);
-		if (handle->_msg_ptr + rr->rdlength > handle->_eom) goto size;
+		if (rr->rdlength > handle->_eom - handle->_msg_ptr) goto size;
 		rr->rdata = handle->_msg_ptr;
 		handle->_msg_ptr += rr->rdlength;
 	} else {
@@ -159,13 +163,11 @@
 	return -1;
 }
 
-int __dn_expand(const unsigned char *, const unsigned char *, const unsigned char *, char *, int);
-
-int ns_name_uncompress(const u_char *msg, const u_char *eom,
-		       const u_char *src, char *dst, size_t dstsiz)
+int ns_name_uncompress(const unsigned char *msg, const unsigned char *eom,
+                       const unsigned char *src, char *dst, size_t dstsiz)
 {
 	int r;
-	r = __dn_expand(msg, eom, src, dst, dstsiz);
+	r = dn_expand(msg, eom, src, dst, dstsiz);
 	if (r < 0) errno = EMSGSIZE;
 	return r;
 }

[-- Attachment #3: ns_parse.c --]
[-- Type: text/plain, Size: 3965 bytes --]

#define _BSD_SOURCE
#include <errno.h>
#include <stddef.h>
#include <resolv.h>
#include <arpa/nameser.h>

const struct _ns_flagdata _ns_flagdata[16] = {
	{ 0x8000, 15 },
	{ 0x7800, 11 },
	{ 0x0400, 10 },
	{ 0x0200, 9 },
	{ 0x0100, 8 },
	{ 0x0080, 7 },
	{ 0x0040, 6 },
	{ 0x0020, 5 },
	{ 0x0010, 4 },
	{ 0x000f, 0 },
	{ 0x0000, 0 },
	{ 0x0000, 0 },
	{ 0x0000, 0 },
	{ 0x0000, 0 },
	{ 0x0000, 0 },
	{ 0x0000, 0 },
};

unsigned ns_get16(const unsigned char *cp)
{
	return cp[0]<<8 | cp[1];
}

unsigned long ns_get32(const unsigned char *cp)
{
	return (unsigned)cp[0]<<24 | cp[1]<<16 | cp[2]<<8 | cp[3];
}

void ns_put16(unsigned s, unsigned char *cp)
{
	*cp++ = s>>8;
	*cp++ = s;
}

void ns_put32(unsigned long l, unsigned char *cp)
{
	*cp++ = l>>24;
	*cp++ = l>>16;
	*cp++ = l>>8;
	*cp++ = l;
}

int ns_initparse(const unsigned char *msg, int msglen, ns_msg *handle)
{
	int i, r;

	handle->_msg = msg;
	handle->_eom = msg + msglen;
	if (msglen < (2 + ns_s_max) * NS_INT16SZ) goto bad;
	NS_GET16(handle->_id, msg);
	NS_GET16(handle->_flags, msg);
	for (i = 0; i < ns_s_max; i++) {
		if (NS_INT16SZ > handle->_eom - msg) goto bad;
		NS_GET16(handle->_counts[i], msg);
	}
	for (i = 0; i < ns_s_max; i++) {
		if (handle->_counts[i]) {
			handle->_sections[i] = msg;
			r = ns_skiprr(msg, handle->_eom, i, handle->_counts[i]);
			if (r < 0) return -1;
			msg += r;
		} else {
			handle->_sections[i] = NULL;
		}
	}
	if (msg != handle->_eom) goto bad;
	handle->_sect = ns_s_max;
	handle->_rrnum = -1;
	handle->_msg_ptr = NULL;
	return 0;
bad:
	errno = EMSGSIZE;
	return -1;
}

int ns_skiprr(const unsigned char *ptr, const unsigned char *eom, ns_sect section, int count)
{
	const unsigned char *p = ptr;
	int r;

	while (count--) {
		r = dn_skipname(p, eom);
		if (r < 0) goto bad;
		if (r + 2 * NS_INT16SZ > eom - p) goto bad;
		p += r + 2 * NS_INT16SZ;
		if (section != ns_s_qd) {
			if (NS_INT32SZ + NS_INT16SZ > eom - p) goto bad;
			p += NS_INT32SZ;
			NS_GET16(r, p);
			if (r > eom - p) goto bad;
			p += r;
		}
	}
	return ptr - p;
bad:
	errno = EMSGSIZE;
	return -1;
}

int ns_parserr(ns_msg *handle, ns_sect section, int rrnum, ns_rr *rr)
{
	int r;

	if (section < 0 || section >= ns_s_max) goto bad;
	if (section != handle->_sect) {
		handle->_sect = section;
		handle->_rrnum = 0;
		handle->_msg_ptr = handle->_sections[section];
	}
	if (rrnum == -1) rrnum = handle->_rrnum;
	if (rrnum < 0 || rrnum >= handle->_counts[section]) goto bad;
	if (rrnum < handle->_rrnum) {
		handle->_rrnum = 0;
		handle->_msg_ptr = handle->_sections[section];
	}
	if (rrnum > handle->_rrnum) {
		r = ns_skiprr(handle->_msg_ptr, handle->_eom, section, rrnum - handle->_rrnum);
		if (r < 0) return -1;
		handle->_msg_ptr += r;
		handle->_rrnum = rrnum;
	}
	r = dn_expand(handle->_msg, handle->_eom, handle->_msg_ptr, rr->name, NS_MAXDNAME);
	if (r < 0) return -1;
	handle->_msg_ptr += r;
	if (2 * NS_INT16SZ > handle->_eom - handle->_msg_ptr) goto size;
	NS_GET16(rr->type, handle->_msg_ptr);
	NS_GET16(rr->rr_class, handle->_msg_ptr);
	if (section != ns_s_qd) {
		if (NS_INT32SZ + NS_INT16SZ > handle->_eom - handle->_msg_ptr) goto size;
		NS_GET32(rr->ttl, handle->_msg_ptr);
		NS_GET16(rr->rdlength, handle->_msg_ptr);
		if (rr->rdlength > handle->_eom - handle->_msg_ptr) goto size;
		rr->rdata = handle->_msg_ptr;
		handle->_msg_ptr += rr->rdlength;
	} else {
		rr->ttl = 0;
		rr->rdlength = 0;
		rr->rdata = NULL;
	}
	handle->_rrnum++;
	if (handle->_rrnum > handle->_counts[section]) {
		handle->_sect = section + 1;
		if (handle->_sect == ns_s_max) {
			handle->_rrnum = -1;
			handle->_msg_ptr = NULL;
		} else {
			handle->_rrnum = 0;
		}
	}
	return 0;
bad:
	errno = ENODEV;
	return -1;
size:
	errno = EMSGSIZE;
	return -1;
}

int ns_name_uncompress(const unsigned char *msg, const unsigned char *eom,
                       const unsigned char *src, char *dst, size_t dstsiz)
{
	int r;
	r = dn_expand(msg, eom, src, dst, dstsiz);
	if (r < 0) errno = EMSGSIZE;
	return r;
}


             reply	other threads:[~2014-12-14  0:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-14  0:43 Rich Felker [this message]
2014-12-14  7:38 ` Felix Janda
2014-12-14 17:23   ` Rich Felker
2014-12-14 19:05     ` Felix Janda
2014-12-14 22:56       ` Rich Felker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141214004320.GA17102@brightrain.aerifal.cx \
    --to=dalias@libc.org \
    --cc=musl@lists.openwall.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).