From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/6718 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Merging ns_parse from Alpine Date: Sat, 13 Dec 2014 19:43:20 -0500 Message-ID: <20141214004320.GA17102@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="sm4nu43k4a2Rpi4c" X-Trace: ger.gmane.org 1418517825 24442 80.91.229.3 (14 Dec 2014 00:43:45 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 14 Dec 2014 00:43:45 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-6731-gllmg-musl=m.gmane.org@lists.openwall.com Sun Dec 14 01:43:38 2014 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1XzxHd-0003zH-0L for gllmg-musl@m.gmane.org; Sun, 14 Dec 2014 01:43:37 +0100 Original-Received: (qmail 1468 invoked by uid 550); 14 Dec 2014 00:43:35 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 1460 invoked from network); 14 Dec 2014 00:43:34 -0000 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:6718 Archived-At: --sm4nu43k4a2Rpi4c Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 --sm4nu43k4a2Rpi4c Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="ns_parse_changes_v1.diff" --- 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; } --sm4nu43k4a2Rpi4c Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="ns_parse.c" #define _BSD_SOURCE #include #include #include #include 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; } --sm4nu43k4a2Rpi4c--