Reviewed by: freebsd-current

    Add bounds checking to netbios NS packet resolving code.  This should
    prevent natd from crashing on badly formed netbios packets (as might be
    heard when the machine is sitting on a cable modem or certain DSL
    networks), and also closes potential security holes that might have
    exploited the lack of bounds checking in the previous version of the
    code.

3 files changed, 192 insertions, 44 deletions

diff --git a/sys/netinet/libalias/alias.c b/sys/netinet/libalias/alias.c
index 48a852a..50e597f 100644
--- a/sys/netinet/libalias/alias.c
+++ b/sys/netinet/libalias/alias.c
@@ -73,6 +73,9 @@
         - Eliminated PacketAliasIn2() and
           PacketAliasOut2() as poorly conceived.
 
+    Version 2.3 Dec 1998 (dillon)
+	- Major bounds checking additions, see FreeBSD/CVS
+
     See HISTORY file for additional revisions.
 
 */
@@ -603,6 +606,7 @@ UdpAliasIn(struct ip *pip)
         u_short alias_port;
         int accumulate;
         u_short *sptr;
+	int r = 0;
 
         alias_address = GetAliasAddress(link);
         original_address = GetOriginalAddress(link);
@@ -613,11 +617,11 @@ UdpAliasIn(struct ip *pip)
 		if (ntohs(ud->uh_dport) == NETBIOS_DGM_PORT_NUMBER
          || ntohs(ud->uh_sport) == NETBIOS_DGM_PORT_NUMBER )
 		{
-            AliasHandleUdpNbt(pip, link, &original_address, ud->uh_dport);
+            r = AliasHandleUdpNbt(pip, link, &original_address, ud->uh_dport);
 		} else if (ntohs(ud->uh_dport) == NETBIOS_NS_PORT_NUMBER
          || ntohs(ud->uh_sport) == NETBIOS_NS_PORT_NUMBER )
 		{
-            AliasHandleUdpNbtNS(pip, link, 
+            r = AliasHandleUdpNbtNS(pip, link, 
 								&alias_address,
 								&alias_port,
 								&original_address, 
@@ -648,7 +652,14 @@ UdpAliasIn(struct ip *pip)
                              (u_short *) &pip->ip_dst,
                              2);
         pip->ip_dst = original_address;
-        return(PKT_ALIAS_OK);
+
+	/*
+	 * If we cannot figure out the packet, ignore it.
+	 */
+	if (r < 0)
+	    return(PKT_ALIAS_IGNORED);
+	else
+	    return(PKT_ALIAS_OK);
     }
     return(PKT_ALIAS_IGNORED);
 }
diff --git a/sys/netinet/libalias/alias_local.h b/sys/netinet/libalias/alias_local.h
index 0e94a64..3846281 100644
--- a/sys/netinet/libalias/alias_local.h
+++ b/sys/netinet/libalias/alias_local.h
@@ -90,8 +90,8 @@ void HouseKeeping(void);
 /*lint -save -library Suppress flexelint warnings */
 void AliasHandleFtpOut(struct ip *, struct alias_link *, int);
 void AliasHandleIrcOut(struct ip *pip, struct alias_link *link, int maxsize );
-void AliasHandleUdpNbt(struct ip *, struct alias_link *, struct in_addr *, u_short);
-void AliasHandleUdpNbtNS(struct ip *, struct alias_link *, struct in_addr *, u_short *, struct in_addr *, u_short *);
+int AliasHandleUdpNbt(struct ip *, struct alias_link *, struct in_addr *, u_short);
+int AliasHandleUdpNbtNS(struct ip *, struct alias_link *, struct in_addr *, u_short *, struct in_addr *, u_short *);
 void AliasHandleCUSeeMeOut(struct ip *, struct alias_link *);
 void AliasHandleCUSeeMeIn(struct ip *, struct in_addr);
 
diff --git a/sys/netinet/libalias/alias_nbt.c b/sys/netinet/libalias/alias_nbt.c
index f3581a7..cd07431 100644
--- a/sys/netinet/libalias/alias_nbt.c
+++ b/sys/netinet/libalias/alias_nbt.c
@@ -15,7 +15,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id:$
+ * $Id: alias_nbt.c,v 1.1 1998/05/24 03:03:10 amurai Exp $
  *
  *  TODO:
  *       oClean up. 
@@ -132,15 +132,21 @@ void PrintRcode( u_char rcode )  {
 
 
 /* Handling Name field */
-u_char *AliasHandleName ( u_char *p ) {
+u_char *AliasHandleName ( u_char *p, char *pmax ) {
 
 	u_char *s;
 	u_char c;
 	int		compress;
 
 	/* Following length field */
+
+	if (p == NULL || (char *)p >= pmax)
+		return(NULL);
+
 	if (*p & 0xc0 ) {
 		p = p + 2;
+		if ((char *)p > pmax)
+			return(NULL);
 		return ((u_char *)p);
 	}
 	while ( ( *p & 0x3f) != 0x00 ) {
@@ -152,6 +158,10 @@ u_char *AliasHandleName ( u_char *p ) {
 		
 	 	/* Get next length field */
 		p = (u_char *)(p + (*p & 0x3f) + 1);
+		if ((char *)p > pmax) {
+			p = NULL;
+			break;
+		}
 #ifdef DEBUG
 		printf(":");
 #endif
@@ -179,7 +189,10 @@ u_char *AliasHandleName ( u_char *p ) {
     }
 
 	/* Set up to out of Name field */
-	p++;
+	if (p == NULL || (char *)p >= pmax)
+	    p = NULL;
+	else
+	    p++;
 	return ((u_char *)p);
 }
 
@@ -194,19 +207,24 @@ u_char *AliasHandleName ( u_char *p ) {
 #define DGM_POSITIVE_RES	0x15
 #define DGM_NEGATIVE_RES	0x16
 
-void AliasHandleUdpNbt(
+int AliasHandleUdpNbt(
 	struct ip 		  	*pip,	 /* IP packet to examine/patch */
 	struct alias_link 	*link,
 	struct in_addr		*alias_address,
-	u_short 			alias_port )
-{
+    u_short 		alias_port
+) {
     struct udphdr *	uh;
     NbtDataHeader 	*ndh;
-	u_char			*p;
+    u_char		*p = NULL;
+    char		*pmax;
         
     /* Calculate data length of UDP packet */
     uh =  (struct udphdr *) ((char *) pip + (pip->ip_hl << 2));
+    pmax = (char *)uh + ntohs( uh->uh_ulen );
+
 	ndh = (NbtDataHeader *)((char *)uh + (sizeof (struct udphdr)));
+    if ((char *)(ndh + 1) > pmax)
+	    return(-1);
 #ifdef DEBUG
 	printf("\nType=%02x,", ndh->type );
 #endif
@@ -215,8 +233,8 @@ void AliasHandleUdpNbt(
 		case DGM_DIRECT_GROUP:
 		case DGM_BROADCAST:
 			p = (u_char *)ndh + 14;
-			p = AliasHandleName ( p ); /* Source Name */
-			p = AliasHandleName ( p ); /* Destination Name */
+		    p = AliasHandleName ( p, pmax ); /* Source Name */
+		    p = AliasHandleName ( p, pmax ); /* Destination Name */
 			break;
 		case DGM_ERROR:
 			p = (u_char *)ndh + 11;
@@ -225,9 +243,11 @@ void AliasHandleUdpNbt(
 		case DGM_POSITIVE_RES:
 		case DGM_NEGATIVE_RES:
 			p = (u_char *)ndh + 10;
-			p = AliasHandleName ( p ); /* Destination Name */
+		    p = AliasHandleName ( p, pmax ); /* Destination Name */
 			break;
 	}
+    if (p == NULL || (char *)p > pmax)
+	    p = NULL;
 #ifdef DEBUG
 	printf("%s:%d-->", inet_ntoa(ndh->source_ip), ntohs(ndh->source_port) );
 #endif
@@ -251,6 +271,7 @@ void AliasHandleUdpNbt(
 	printf("%s:%d\n", inet_ntoa(ndh->source_ip), ntohs(ndh->source_port) );
 	fflush(stdout);
 #endif
+    return((p == NULL) ? -1 : 0);
 }
 /* Question Section */
 #define QS_TYPE_NB		0x0020
@@ -261,14 +282,22 @@ typedef struct {
 	u_short	class;	/* The class of Request */
 } NBTNsQuestion;
 
-u_char *AliasHandleQuestion(u_short count,
+u_char *
+AliasHandleQuestion(
+    u_short count,
 							NBTNsQuestion *q,
+    char *pmax,
 							NBTArguments  *nbtarg)
 {
 
 	while ( count != 0 ) {
 		/* Name Filed */
-		q = (NBTNsQuestion *)AliasHandleName((u_char *)q );
+		q = (NBTNsQuestion *)AliasHandleName((u_char *)q, pmax);
+
+		if (q == NULL || (char *)(q + 1) > pmax) {
+			q = NULL;
+			break;
+		}
 
 		/* Type and Class filed */
 		switch ( ntohs(q->type) ) {
@@ -308,12 +337,17 @@ typedef struct {
 	struct	in_addr	addr;
 } NBTNsRNB;
 
-u_char *AliasHandleResourceNB( NBTNsResource *q,
+u_char *
+AliasHandleResourceNB( 
+    NBTNsResource *q,
+    char *pmax, 
 							   NBTArguments  *nbtarg)
 {
 	NBTNsRNB	*nb;
 	u_short bcount;
 
+	if (q == NULL || (char *)(q + 1) > pmax)
+		return(NULL);
 	/* Check out a length */
 	bcount = ntohs(q->rdlen);
 
@@ -325,7 +359,11 @@ u_char *AliasHandleResourceNB( NBTNsResource *q,
 	printf("NB rec[%s", inet_ntoa(nbtarg->oldaddr));
             printf("->%s, %dbytes] ",inet_ntoa(nbtarg->newaddr ), bcount);
 #endif
-	while ( bcount != 0 )  {
+	while ( nb != NULL && bcount != 0 )  {
+		if ((char *)(nb + 1) > pmax) {
+			nb = NULL;
+			break;
+		}
 #ifdef DEBUG
 		printf("<%s>", inet_ntoa(nb->addr) );
 #endif
@@ -356,6 +394,9 @@ u_char *AliasHandleResourceNB( NBTNsResource *q,
 		nb=(NBTNsRNB *)((u_char *)nb + SizeOfNsRNB);
 	 	bcount -= SizeOfNsRNB;
 	}
+	if (nb == NULL || (char *)(nb + 1) > pmax) {
+		nb = NULL;
+	}
 
 	return ((u_char *)nb);
 }
@@ -365,12 +406,18 @@ typedef struct {
 	struct	in_addr	addr;
 } NBTNsResourceA;
 
-u_char *AliasHandleResourceA( NBTNsResource *q,
+u_char *
+AliasHandleResourceA( 
+    NBTNsResource *q,
+    char *pmax,
 						 	  NBTArguments  *nbtarg)
 {
 	NBTNsResourceA	*a;
 	u_short bcount;
 
+	if (q == NULL || (char *)(q + 1) > pmax)
+		return(NULL);
+
 	/* Forward to Resource A position */
 	a = (NBTNsResourceA *)( (u_char *)q + sizeof(NBTNsResource) );
 
@@ -383,6 +430,8 @@ u_char *AliasHandleResourceA( NBTNsResource *q,
         printf("->%s]",inet_ntoa(nbtarg->newaddr ));
 #endif
 	while ( bcount != 0 )  {
+		if (a == NULL || (char *)(a + 1) > pmax)
+			return(NULL);
 #ifdef DEBUG
 		printf("..%s", inet_ntoa(a->addr) );
 #endif
@@ -405,6 +454,8 @@ u_char *AliasHandleResourceA( NBTNsResource *q,
 		a++;	/*XXXX*/
 		bcount -= SizeOfResourceA;
 	}
+	if (a == NULL || (char *)(a + 1) > pmax)
+		a =  NULL;
 	return ((u_char *)a);
 }
 
@@ -412,12 +463,18 @@ typedef struct {
 	u_short opcode:4, flags:8, resv:4;
 } NBTNsResourceNULL;
 
-u_char *AliasHandleResourceNULL( NBTNsResource *q,
+u_char *
+AliasHandleResourceNULL( 
+    NBTNsResource *q, 
+    char *pmax,
 						 	     NBTArguments  *nbtarg)
 {
 	NBTNsResourceNULL	*n;
 	u_short bcount;
 
+	if (q == NULL || (char *)(q + 1) > pmax)
+		return(NULL);
+
 	/* Forward to Resource NULL position */
 	n = (NBTNsResourceNULL *)( (u_char *)q + sizeof(NBTNsResource) );
 
@@ -426,19 +483,31 @@ u_char *AliasHandleResourceNULL( NBTNsResource *q,
 
 	/* Processing all in_addr array */
 	while ( bcount != 0 )  {
+		if ((char *)(n + 1) > pmax) {
+			n = NULL;
+			break;
+		}
 		n++;
 		bcount -= sizeof(NBTNsResourceNULL);
 	}
+	if ((char *)(n + 1) > pmax)
+		n = NULL;
 
 	return ((u_char *)n);
 }
 
-u_char *AliasHandleResourceNS( NBTNsResource *q,
+u_char *
+AliasHandleResourceNS( 
+    NBTNsResource *q,
+    char *pmax,
 						 	     NBTArguments  *nbtarg)
 {
 	NBTNsResourceNULL	*n;
 	u_short bcount;
 
+	if (q == NULL || (char *)(q + 1) > pmax)
+		return(NULL);
+
 	/* Forward to Resource NULL position */
 	n = (NBTNsResourceNULL *)( (u_char *)q + sizeof(NBTNsResource) );
 
@@ -446,8 +515,11 @@ u_char *AliasHandleResourceNS( NBTNsResource *q,
 	bcount = ntohs(q->rdlen);
 
 	/* Resource Record Name Filed */
-	q = (NBTNsResource *)AliasHandleName( (u_char *)n ); /* XXX */
+	q = (NBTNsResource *)AliasHandleName( (u_char *)n, pmax ); /* XXX */
 
+	if (q == NULL || (char *)((u_char *)n + bcount) > pmax)
+		return(NULL);
+	else
 	return ((u_char *)n + bcount);
 }
 
@@ -455,28 +527,44 @@ typedef struct {
 	u_short	numnames;
 } NBTNsResourceNBSTAT;
 
-u_char *AliasHandleResourceNBSTAT( NBTNsResource *q,
+u_char *
+AliasHandleResourceNBSTAT(
+    NBTNsResource *q,
+    char *pmax,
 						 	       NBTArguments  *nbtarg)
 {
 	NBTNsResourceNBSTAT	*n;
 	u_short bcount;
 
+	if (q == NULL || (char *)(q + 1) > pmax)
+		return(NULL);
+
 	/* Forward to Resource NBSTAT position */
 	n = (NBTNsResourceNBSTAT *)( (u_char *)q + sizeof(NBTNsResource) );
 
 	/* Check out of length */
 	bcount = ntohs(q->rdlen);
 
+	if (q == NULL || (char *)((u_char *)n + bcount) > pmax)
+		return(NULL);
+	else
 	return ((u_char *)n + bcount);
 }
 
-u_char *AliasHandleResource(u_short count,
+u_char *
+AliasHandleResource(
+    u_short count, 
 							NBTNsResource *q,
-							NBTArguments  *nbtarg)
+    char *pmax,
+    NBTArguments  
+    *nbtarg)
 {
 	while ( count != 0 ) {
 		/* Resource Record Name Filed */
-		q = (NBTNsResource *)AliasHandleName( (u_char *)q );
+		q = (NBTNsResource *)AliasHandleName( (u_char *)q, pmax );
+
+		if (q == NULL || (char *)(q + 1) > pmax)
+			break;
 #ifdef DEBUG
 		printf("type=%02x, count=%d\n", ntohs(q->type), count );
 #endif
@@ -484,22 +572,45 @@ u_char *AliasHandleResource(u_short count,
 		/* Type and Class filed */
 		switch ( ntohs(q->type) ) {
 			case RR_TYPE_NB:
-				q = (NBTNsResource *)AliasHandleResourceNB( q, nbtarg );
+				q = (NBTNsResource *)AliasHandleResourceNB( 
+				    q,
+				    pmax,
+				    nbtarg 
+				);
 				break;
 			case RR_TYPE_A: 
-				q = (NBTNsResource *)AliasHandleResourceA( q, nbtarg );
+				q = (NBTNsResource *)AliasHandleResourceA( 
+				    q, 
+				    pmax, 
+				    nbtarg
+				);
 				break;
 			case RR_TYPE_NS:
-				q = (NBTNsResource *)AliasHandleResourceNS( q, nbtarg );
+				q = (NBTNsResource *)AliasHandleResourceNS( 
+				    q,
+				    pmax, 
+				    nbtarg 
+				);
 				break;
 			case RR_TYPE_NULL:
-				q = (NBTNsResource *)AliasHandleResourceNULL( q, nbtarg );
+				q = (NBTNsResource *)AliasHandleResourceNULL( 
+				    q, 
+				    pmax, 
+				    nbtarg 
+				);
 				break;
 			case RR_TYPE_NBSTAT:
-				q = (NBTNsResource *)AliasHandleResourceNBSTAT( q, nbtarg );
+				q = (NBTNsResource *)AliasHandleResourceNBSTAT(
+				    q,
+				    pmax, 
+				    nbtarg
+				);
 				break;
-			default: printf("\nUnknown Type of Resource %0x\n",
-								 ntohs(q->type) );
+			default: 
+				printf(
+				    "\nUnknown Type of Resource %0x\n", 
+				    ntohs(q->type) 
+				);
 				break;
 		}
 		count--;
@@ -508,7 +619,7 @@ u_char *AliasHandleResource(u_short count,
 	return ((u_char *)q);
 }
 
-void AliasHandleUdpNbtNS(
+int AliasHandleUdpNbtNS(
 	struct ip 		  	*pip,	 /* IP packet to examine/patch */
 	struct alias_link 	*link,
 	struct in_addr		*alias_address,
@@ -518,8 +629,8 @@ void AliasHandleUdpNbtNS(
 {
     struct udphdr *	uh;
 	NbtNSHeader	  * nsh;
-	u_short		    dlen;
 	u_char		  * p;
+	char		*pmax;
 	NBTArguments    nbtarg;
 
 	/* Set up Common Parameter */	
@@ -531,12 +642,16 @@ void AliasHandleUdpNbtNS(
     /* Calculate data length of UDP packet */
     uh =  (struct udphdr *) ((char *) pip + (pip->ip_hl << 2));
 	nbtarg.uh_sum	=	&(uh->uh_sum);
-    dlen = ntohs( uh->uh_ulen );
 	nsh = (NbtNSHeader *)((char *)uh + (sizeof(struct udphdr)));
 	p = (u_char *)(nsh + 1);
+    pmax = (char *)uh + ntohs( uh->uh_ulen );
+
+    if ((char *)(nsh + 1) > pmax)
+	return(-1);
 
 #ifdef DEBUG
-	printf(" [%s] ID=%02x, op=%01x, flag=%02x, rcode=%01x, qd=%04x, an=%04x, ns=%04x, ar=%04x, [%d]-->", 
+    printf(" [%s] ID=%02x, op=%01x, flag=%02x, rcode=%01x, qd=%04x"
+	   ", an=%04x, ns=%04x, ar=%04x, [%d]-->", 
 		nsh->dir ? "Response": "Request",
 		nsh->nametrid,
 		nsh->opcode,
@@ -546,31 +661,53 @@ void AliasHandleUdpNbtNS(
 		ntohs(nsh->ancount),
 		ntohs(nsh->nscount),
 		ntohs(nsh->arcount),
-		(u_char *)p -(u_char *)nsh);
+	(u_char *)p -(u_char *)nsh
+    );
 #endif
 
 	/* Question Entries */
 	if (ntohs(nsh->qdcount) !=0 ) {
-	p = AliasHandleQuestion(ntohs(nsh->qdcount), (NBTNsQuestion *)p, &nbtarg );
+	p = AliasHandleQuestion(
+	    ntohs(nsh->qdcount),
+	    (NBTNsQuestion *)p,
+	    pmax, 
+	    &nbtarg 
+	);
 	}
 
 	/* Answer Resource Records */
 	if (ntohs(nsh->ancount) !=0 ) {
-	p = AliasHandleResource(ntohs(nsh->ancount), (NBTNsResource *)p, &nbtarg );
+	p = AliasHandleResource(
+	    ntohs(nsh->ancount),
+	    (NBTNsResource *)p,
+	    pmax, 
+	    &nbtarg 
+	);
 	}
 
 	/* Authority Resource Recodrs */
 	if (ntohs(nsh->nscount) !=0 ) {
-	p = AliasHandleResource(ntohs(nsh->nscount), (NBTNsResource *)p, &nbtarg );
+	p = AliasHandleResource(
+	    ntohs(nsh->nscount), 
+	    (NBTNsResource *)p,
+	    pmax, 
+	    &nbtarg 
+	);
 	}
 
 	/* Additional Resource Recodrs */
 	if (ntohs(nsh->arcount) !=0 ) {
-	p = AliasHandleResource(ntohs(nsh->arcount), (NBTNsResource *)p, &nbtarg );
+	p = AliasHandleResource(
+	    ntohs(nsh->arcount),
+	    (NBTNsResource *)p,
+	    pmax, 
+	    &nbtarg 
+	);
 	}
 
 #ifdef DEBUG
 	 	PrintRcode(nsh->rcode);
 #endif
-	return;
+    return ((p == NULL) ? -1 : 0);
 }
+