From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Sat, 3 Dec 2016 14:51:54 +0000 (+0000)
Subject: SECURITY: Defend adns_rr_info (somewhat) from bogus *datap
X-Git-Tag: adns-1.5.2~26
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?a=commitdiff_plain;h=37792aacaf7abbcdac6a02715a5ef794b5147f13;p=adns.git

SECURITY: Defend adns_rr_info (somewhat) from bogus *datap

The general pattern for formatting integers is to sprintf into a
fixed-size buffer.  This is correct if the input is in the right
range; if it isn't, the buffer may be overrun (depending on the sizes
of the types on the current platform).

Of course the inputs ought to be right.  And there are pointers in
there too, so perhaps we could say that the caller ought to check
these things.  I think it's better to require the caller to make the
pointer structure right, but to have the code here be defensive about
(and tolerate with an erro but without crashing) out-of-range integer
values.

So: defend each of these integer conversion sites with a check for the
actual permitted range, and return adns_s_invaliddata if not.

The lack of this check causes the SOA sign extension bug to be a
serious security problem: the sign extended SOA value is out of range,
and will overrun the buffer when reconverted.

Found by AFL 2.35b.  CVE-2017-9106.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---

diff --git a/src/types.c b/src/types.c
index 569962e..65b9065 100644
--- a/src/types.c
+++ b/src/types.c
@@ -1111,6 +1111,10 @@ static void mf_inthostaddr(adns_query qu, void *datap) {
 static adns_status csp_intofinthost(vbuf *vb, int i) {
   char buf[10];
 
+  if (i < 0 || i > 0xffff)
+    /* currently only used for MX whose priorities are 16-bit */
+    return adns_s_invaliddata;
+  
   sprintf(buf,"%u ",i);
   CSP_ADDSTR(buf);
   return adns_s_ok;
@@ -1413,6 +1417,8 @@ static adns_status cs_soa(vbuf *vb, const void *datap) {
   st= csp_mailbox(vb,rrp->rname);  if (st) return st;
 
   for (i=0; i<5; i++) {
+    if (rrp->serial > 0xffffffffUL)
+      return adns_s_invaliddata;
     sprintf(buf," %lu",(&rrp->serial)[i]);
     CSP_ADDSTR(buf);
   }
@@ -1501,6 +1507,10 @@ static int di_srv(adns_state ads, const void *datap_a, const void *datap_b) {
 static adns_status csp_srv_begin(vbuf *vb, const adns_rr_srvha *rrp
 				   /* might be adns_rr_srvraw* */) {
   char buf[30];
+  if (rrp->priority < 0 || rrp->priority > 0xffff ||
+      rrp->weight   < 0 || rrp->weight   > 0xffff ||
+      rrp->port     < 0 || rrp->port     > 0xffff)
+    return adns_s_invaliddata;
   sprintf(buf,"%u %u %u ", rrp->priority, rrp->weight, rrp->port);
   CSP_ADDSTR(buf);
   return adns_s_ok;
@@ -1616,6 +1626,9 @@ static adns_status cs_opaque(vbuf *vb, const void *datap) {
   int l;
   unsigned char *p;
 
+  if (rrp->len < 0 || rrp->len > 0xffff)
+    return adns_s_invaliddata;
+
   sprintf(buf,"\\# %d",rrp->len);
   CSP_ADDSTR(buf);