365 lines
		
	
	
		
			10 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
			
		
		
	
	
			365 lines
		
	
	
		
			10 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
| From 6689e336042d2ca0b075f8db1fe30ed6b47c7d4c Mon Sep 17 00:00:00 2001
 | |
| From: Simon Kelley <simon@thekelleys.org.uk>
 | |
| Date: Wed, 11 Nov 2020 23:25:04 +0000
 | |
| Subject: [PATCH 1/4] Fix remote buffer overflow CERT VU#434904
 | |
| 
 | |
| The problem is in the sort_rrset() function and allows a remote
 | |
| attacker to overwrite memory. Any dnsmasq instance with DNSSEC
 | |
| enabled is vulnerable.
 | |
| ---
 | |
|  src/dnssec.c | 273 ++++++++++++++++++++++++++++-----------------------
 | |
|  1 file changed, 152 insertions(+), 121 deletions(-)
 | |
| 
 | |
| diff --git a/src/dnssec.c b/src/dnssec.c
 | |
| index 8143185..0c703ac 100644
 | |
| --- a/src/dnssec.c
 | |
| +++ b/src/dnssec.c
 | |
| @@ -222,138 +222,147 @@ static int check_date_range(u32 date_start, u32 date_end)
 | |
|      && serial_compare_32(curtime, date_end) == SERIAL_LT;
 | |
|  }
 | |
|  
 | |
| -/* Return bytes of canonicalised rdata, when the return value is zero, the remaining 
 | |
| -   data, pointed to by *p, should be used raw. */
 | |
| -static int get_rdata(struct dns_header *header, size_t plen, unsigned char *end, char *buff, int bufflen,
 | |
| -		     unsigned char **p, u16 **desc)
 | |
| +/* Return bytes of canonicalised rrdata one by one.
 | |
| +   Init state->ip with the RR, and state->end with the end of same.
 | |
| +   Init state->op to NULL.
 | |
| +   Init state->desc to RR descriptor.
 | |
| +   Init state->buff with a MAXDNAME * 2 buffer.
 | |
| +   
 | |
| +   After each call which returns 1, state->op points to the next byte of data.
 | |
| +   On returning 0, the end has been reached.
 | |
| +*/
 | |
| +struct rdata_state {
 | |
| +  u16 *desc;
 | |
| +  size_t c;
 | |
| +  unsigned char *end, *ip, *op;
 | |
| +  char *buff;
 | |
| +};
 | |
| +
 | |
| +static int get_rdata(struct dns_header *header, size_t plen, struct rdata_state *state)
 | |
|  {
 | |
| -  int d = **desc;
 | |
| +  int d;
 | |
|    
 | |
| -  /* No more data needs mangling */
 | |
| -  if (d == (u16)-1)
 | |
| +  if (state->op && state->c != 1)
 | |
|      {
 | |
| -      /* If there's more data than we have space for, just return what fits,
 | |
| -	 we'll get called again for more chunks */
 | |
| -      if (end - *p > bufflen)
 | |
| -	{
 | |
| -	  memcpy(buff, *p, bufflen);
 | |
| -	  *p += bufflen;
 | |
| -	  return bufflen;
 | |
| -	}
 | |
| -      
 | |
| -      return 0;
 | |
| +      state->op++;
 | |
| +      state->c--;
 | |
| +      return 1;
 | |
|      }
 | |
| - 
 | |
| -  (*desc)++;
 | |
| -  
 | |
| -  if (d == 0 && extract_name(header, plen, p, buff, 1, 0))
 | |
| -    /* domain-name, canonicalise */
 | |
| -    return to_wire(buff);
 | |
| -  else
 | |
| -    { 
 | |
| -      /* plain data preceding a domain-name, don't run off the end of the data */
 | |
| -      if ((end - *p) < d)
 | |
| -	d = end - *p;
 | |
| +
 | |
| +  while (1)
 | |
| +    {
 | |
| +      d = *(state->desc);
 | |
|        
 | |
| -      if (d != 0)
 | |
| +      if (d == (u16)-1)
 | |
|  	{
 | |
| -	  memcpy(buff, *p, d);
 | |
| -	  *p += d;
 | |
| +	  /* all the bytes to the end. */
 | |
| +	  if ((state->c = state->end - state->ip) != 0)
 | |
| +	    {
 | |
| +	      state->op = state->ip;
 | |
| +	      state->ip = state->end;;
 | |
| +	    }
 | |
| +	  else
 | |
| +	    return 0;
 | |
| +	}
 | |
| +      else
 | |
| +	{
 | |
| +	  state->desc++;
 | |
| +	  
 | |
| +	  if (d == (u16)0)
 | |
| +	    {
 | |
| +	      /* domain-name, canonicalise */
 | |
| +	      int len;
 | |
| +	      
 | |
| +	      if (!extract_name(header, plen, &state->ip, state->buff, 1, 0) ||
 | |
| +		  (len = to_wire(state->buff)) == 0)
 | |
| +		continue;
 | |
| +	      
 | |
| +	      state->c = len;
 | |
| +	      state->op = (unsigned char *)state->buff;
 | |
| +	    }
 | |
| +	  else
 | |
| +	    {
 | |
| +	      /* plain data preceding a domain-name, don't run off the end of the data */
 | |
| +	      if ((state->end - state->ip) < d)
 | |
| +		d = state->end - state->ip;
 | |
| +	      
 | |
| +	      if (d == 0)
 | |
| +		continue;
 | |
| +		  
 | |
| +	      state->op = state->ip;
 | |
| +	      state->c = d;
 | |
| +	      state->ip += d;
 | |
| +	    }
 | |
|  	}
 | |
|        
 | |
| -      return d;
 | |
| +      return 1;
 | |
|      }
 | |
|  }
 | |
|  
 | |
| -/* Bubble sort the RRset into the canonical order. 
 | |
| -   Note that the byte-streams from two RRs may get unsynced: consider 
 | |
| -   RRs which have two domain-names at the start and then other data.
 | |
| -   The domain-names may have different lengths in each RR, but sort equal
 | |
| -
 | |
| -   ------------
 | |
| -   |abcde|fghi|
 | |
| -   ------------
 | |
| -   |abcd|efghi|
 | |
| -   ------------
 | |
| -
 | |
| -   leaving the following bytes as deciding the order. Hence the nasty left1 and left2 variables.
 | |
| -*/
 | |
| +/* Bubble sort the RRset into the canonical order. */
 | |
|  
 | |
|  static int sort_rrset(struct dns_header *header, size_t plen, u16 *rr_desc, int rrsetidx, 
 | |
|  		      unsigned char **rrset, char *buff1, char *buff2)
 | |
|  {
 | |
| -  int swap, quit, i, j;
 | |
| +  int swap, i, j;
 | |
|    
 | |
|    do
 | |
|      {
 | |
|        for (swap = 0, i = 0; i < rrsetidx-1; i++)
 | |
|  	{
 | |
| -	  int rdlen1, rdlen2, left1, left2, len1, len2, len, rc;
 | |
| -	  u16 *dp1, *dp2;
 | |
| -	  unsigned char *end1, *end2;
 | |
| +	  int rdlen1, rdlen2;
 | |
| +	  struct rdata_state state1, state2;
 | |
| +	  
 | |
|  	  /* Note that these have been determined to be OK previously,
 | |
|  	     so we don't need to check for NULL return here. */
 | |
| -	  unsigned char *p1 = skip_name(rrset[i], header, plen, 10);
 | |
| -	  unsigned char *p2 = skip_name(rrset[i+1], header, plen, 10);
 | |
| -	  
 | |
| -	  p1 += 8; /* skip class, type, ttl */
 | |
| -	  GETSHORT(rdlen1, p1);
 | |
| -	  end1 = p1 + rdlen1;
 | |
| -	  
 | |
| -	  p2 += 8; /* skip class, type, ttl */
 | |
| -	  GETSHORT(rdlen2, p2);
 | |
| -	  end2 = p2 + rdlen2; 
 | |
| +	  state1.ip = skip_name(rrset[i], header, plen, 10);
 | |
| +	  state2.ip = skip_name(rrset[i+1], header, plen, 10);
 | |
| +	  state1.op = state2.op = NULL;
 | |
| +	  state1.buff = buff1;
 | |
| +	  state2.buff = buff2;
 | |
| +	  state1.desc = state2.desc = rr_desc;
 | |
|  	  
 | |
| -	  dp1 = dp2 = rr_desc;
 | |
| +	  state1.ip += 8; /* skip class, type, ttl */
 | |
| +	  GETSHORT(rdlen1, state1.ip);
 | |
| +	  if (!CHECK_LEN(header, state1.ip, plen, rdlen1))
 | |
| +	    return rrsetidx; /* short packet */
 | |
| +	  state1.end = state1.ip + rdlen1;
 | |
|  	  
 | |
| -	  for (quit = 0, left1 = 0, left2 = 0, len1 = 0, len2 = 0; !quit;)
 | |
| +	  state2.ip += 8; /* skip class, type, ttl */
 | |
| +	  GETSHORT(rdlen2, state2.ip);
 | |
| +	  if (!CHECK_LEN(header, state2.ip, plen, rdlen2))
 | |
| +	    return rrsetidx; /* short packet */
 | |
| +	  state2.end = state2.ip + rdlen2; 
 | |
| +	  	  
 | |
| +	  while (1)
 | |
|  	    {
 | |
| -	      if (left1 != 0)
 | |
| -		memmove(buff1, buff1 + len1 - left1, left1);
 | |
| +	      int ok1, ok2;
 | |
|  	      
 | |
| -	      if ((len1 = get_rdata(header, plen, end1, buff1 + left1, (MAXDNAME * 2) - left1, &p1, &dp1)) == 0)
 | |
| -		{
 | |
| -		  quit = 1;
 | |
| -		  len1 = end1 - p1;
 | |
| -		  memcpy(buff1 + left1, p1, len1);
 | |
| -		}
 | |
| -	      len1 += left1;
 | |
| -	      
 | |
| -	      if (left2 != 0)
 | |
| -		memmove(buff2, buff2 + len2 - left2, left2);
 | |
| -	      
 | |
| -	      if ((len2 = get_rdata(header, plen, end2, buff2 + left2, (MAXDNAME *2) - left2, &p2, &dp2)) == 0)
 | |
| -		{
 | |
| -		  quit = 1;
 | |
| -		  len2 = end2 - p2;
 | |
| -		  memcpy(buff2 + left2, p2, len2);
 | |
| -		}
 | |
| -	      len2 += left2;
 | |
| -	       
 | |
| -	      if (len1 > len2)
 | |
| -		left1 = len1 - len2, left2 = 0, len = len2;
 | |
| -	      else
 | |
| -		left2 = len2 - len1, left1 = 0, len = len1;
 | |
| -	      
 | |
| -	      rc = (len == 0) ? 0 : memcmp(buff1, buff2, len);
 | |
| -	      
 | |
| -	      if (rc > 0 || (rc == 0 && quit && len1 > len2))
 | |
| -		{
 | |
| -		  unsigned char *tmp = rrset[i+1];
 | |
| -		  rrset[i+1] = rrset[i];
 | |
| -		  rrset[i] = tmp;
 | |
| -		  swap = quit = 1;
 | |
| -		}
 | |
| -	      else if (rc == 0 && quit && len1 == len2)
 | |
| +	      ok1 = get_rdata(header, plen, &state1);
 | |
| +	      ok2 = get_rdata(header, plen, &state2);
 | |
| +
 | |
| +	      if (!ok1 && !ok2)
 | |
|  		{
 | |
|  		  /* Two RRs are equal, remove one copy. RFC 4034, para 6.3 */
 | |
|  		  for (j = i+1; j < rrsetidx-1; j++)
 | |
|  		    rrset[j] = rrset[j+1];
 | |
|  		  rrsetidx--;
 | |
|  		  i--;
 | |
| +		  break;
 | |
| +		}
 | |
| +	      else if (ok1 && (!ok2 || *state1.op > *state2.op)) 
 | |
| +		{
 | |
| +		  unsigned char *tmp = rrset[i+1];
 | |
| +		  rrset[i+1] = rrset[i];
 | |
| +		  rrset[i] = tmp;
 | |
| +		  swap = 1;
 | |
| +		  break;
 | |
|  		}
 | |
| -	      else if (rc < 0)
 | |
| -		quit = 1;
 | |
| +	      else if (ok2 && (!ok1 || *state2.op > *state1.op))
 | |
| +		break;
 | |
| +	      
 | |
| +	      /* arrive here when bytes are equal, go round the loop again
 | |
| +		 and compare the next ones. */
 | |
|  	    }
 | |
|  	}
 | |
|      } while (swap);
 | |
| @@ -549,15 +558,18 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in
 | |
|        wire_len = to_wire(keyname);
 | |
|        hash->update(ctx, (unsigned int)wire_len, (unsigned char*)keyname);
 | |
|        from_wire(keyname);
 | |
| +
 | |
| +#define RRBUFLEN 300 /* Most RRs are smaller than this. */
 | |
|        
 | |
|        for (i = 0; i < rrsetidx; ++i)
 | |
|  	{
 | |
| -	  int seg;
 | |
| -	  unsigned char *end, *cp;
 | |
| -	  u16 len, *dp;
 | |
| +	  int j;
 | |
| +	  struct rdata_state state;
 | |
| +	  u16 len;
 | |
| +	  unsigned char rrbuf[RRBUFLEN];
 | |
|  	  
 | |
|  	  p = rrset[i];
 | |
| -	 	  
 | |
| +	  
 | |
|  	  if (!extract_name(header, plen, &p, name, 1, 10)) 
 | |
|  	    return STAT_BOGUS;
 | |
|  
 | |
| @@ -566,12 +578,11 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in
 | |
|  	  /* if more labels than in RRsig name, hash *.<no labels in rrsig labels field>  4035 5.3.2 */
 | |
|  	  if (labels < name_labels)
 | |
|  	    {
 | |
| -	      int k;
 | |
| -	      for (k = name_labels - labels; k != 0; k--)
 | |
| +	      for (j = name_labels - labels; j != 0; j--)
 | |
|  		{
 | |
|  		  while (*name_start != '.' && *name_start != 0)
 | |
|  		    name_start++;
 | |
| -		  if (k != 1 && *name_start == '.')
 | |
| +		  if (j != 1 && *name_start == '.')
 | |
|  		    name_start++;
 | |
|  		}
 | |
|  	      
 | |
| @@ -592,24 +603,44 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in
 | |
|  	  if (!CHECK_LEN(header, p, plen, rdlen))
 | |
|  	    return STAT_BOGUS; 
 | |
|  	  
 | |
| -	  end = p + rdlen;
 | |
| +	  /* canonicalise rdata and calculate length of same, use 
 | |
| +	     name buffer as workspace for get_rdata. */
 | |
| +	  state.ip = p;
 | |
| +	  state.op = NULL;
 | |
| +	  state.desc = rr_desc;
 | |
| +	  state.buff = name;
 | |
| +	  state.end = p + rdlen;
 | |
|  	  
 | |
| -	  /* canonicalise rdata and calculate length of same, use name buffer as workspace.
 | |
| -	     Note that name buffer is twice MAXDNAME long in DNSSEC mode. */
 | |
| -	  cp = p;
 | |
| -	  dp = rr_desc;
 | |
| -	  for (len = 0; (seg = get_rdata(header, plen, end, name, MAXDNAME * 2, &cp, &dp)) != 0; len += seg);
 | |
| -	  len += end - cp;
 | |
| -	  len = htons(len);
 | |
| +	  for (j = 0; get_rdata(header, plen, &state); j++)
 | |
| +	    if (j < RRBUFLEN)
 | |
| +	      rrbuf[j] = *state.op;
 | |
| +
 | |
| +	  len = htons((u16)j);
 | |
|  	  hash->update(ctx, 2, (unsigned char *)&len); 
 | |
| +
 | |
| +	  /* If the RR is shorter than RRBUFLEN (most of them, in practice)
 | |
| +	     then we can just digest it now. If it exceeds RRBUFLEN we have to
 | |
| +	     go back to the start and do it in chunks. */
 | |
| +	  if (j >= RRBUFLEN)
 | |
| +	    {
 | |
| +	      state.ip = p;
 | |
| +	      state.op = NULL;
 | |
| +	      state.desc = rr_desc;
 | |
| +
 | |
| +	      for (j = 0; get_rdata(header, plen, &state); j++)
 | |
| +		{
 | |
| +		   rrbuf[j] = *state.op;
 | |
| +
 | |
| +		   if (j == RRBUFLEN - 1)
 | |
| +		     {
 | |
| +		       hash->update(ctx, RRBUFLEN, rrbuf);
 | |
| +		       j = -1;
 | |
| +		     }
 | |
| +		}
 | |
| +	    }
 | |
|  	  
 | |
| -	  /* Now canonicalise again and digest. */
 | |
| -	  cp = p;
 | |
| -	  dp = rr_desc;
 | |
| -	  while ((seg = get_rdata(header, plen, end, name, MAXDNAME * 2, &cp, &dp)))
 | |
| -	    hash->update(ctx, seg, (unsigned char *)name);
 | |
| -	  if (cp != end)
 | |
| -	    hash->update(ctx, end - cp, cp);
 | |
| +	  if (j != 0)
 | |
| +	    hash->update(ctx, j, rrbuf);
 | |
|  	}
 | |
|       
 | |
|        hash->digest(ctx, hash->digest_size, digest);
 | |
| -- 
 | |
| 2.26.2
 | |
| 
 |