Fix buffer overruns.

3 files changed, 60 insertions, 95 deletions

diff --git a/lib/ZParseNot.c b/lib/ZParseNot.c
index 6d393a0..7950e96 100644
--- a/lib/ZParseNot.c
+++ b/lib/ZParseNot.c
@@ -17,55 +17,16 @@ static char rcsid_ZParseNotice_c[] =
 
 #include <internal.h>
 
-/* Assume that strlen is efficient on this machine... */
-#define next_field(ptr)	ptr += strlen (ptr) + 1
-
-#if defined (__GNUC__) && defined (__vax__)
-#undef next_field
-static __inline__ char * Istrend (char *str) {
-    /*
-     * This should be faster on VAX models outside the 2 series.  Don't
-     * use it if you are using MicroVAX 2 servers.  If you are using a
-     * VS2 server, use something like
-     *	#define next_field(ptr)		while(*ptr++)
-     * instead of this code.
-     *
-     * This requires use of GCC to get the optimized code, but
-     * everybody uses GCC, don't they? :-)
-     */
-    register char *str2 asm ("r1");
-    /* Assumes that no field is longer than 64K.... */
-    asm ("locc $0,$65535,(%1)" : "=r" (str2) : "r" (str) : "r0");
-    return str2;
-}
-#define next_field(ptr) ptr = Istrend (ptr) + 1
-#endif
-
-#ifdef mips
-#undef next_field
-/*
- * The compiler doesn't optimize this macro as well as it does the
- * following function.
- */
-#define next_fieldXXX(ptr) do{register unsigned c1,c2;c1= *ptr;	\
-		   while((ptr++,c2= *ptr,c1)&&(ptr++,c1= *ptr,c2));}while(0)
-static char *next_field_1 (s) char *s; {
-    /*
-     * Calling overhead is still present, but this routine is faster
-     * than strlen, and doesn't bother with some of the other math
-     * that we'd just have to undo later anyways.
-     */
-    register unsigned c1 = *s, c2;
-    while (1) {
-	s++; c2 = *s; if (c1 == 0) break;
-	s++; c1 = *s; if (c2 == 0) break;
-	s++; c2 = *s; if (c1 == 0) break;
-	s++; c1 = *s; if (c2 == 0) break;
-    }
-    return s;
+/* Skip to the next NUL-terminated field in the packet. */
+static char *next_field(ptr, end)
+    char *ptr, *end;
+{
+    while (ptr < end && *ptr != '\0')
+	ptr++;
+    if (ptr < end)
+	ptr++;
+    return (ptr);
 }
-#define next_field(ptr)	ptr=next_field_1(ptr)
-#endif
 
 Code_t ZParseNotice(buffer, len, notice)
     char *buffer;
@@ -110,12 +71,12 @@ Code_t ZParseNotice(buffer, len, notice)
     maj = atoi(ptr);
     if (maj != ZVERSIONMAJOR)
 	return (ZERR_VERS);
-    next_field (ptr);
+    ptr = next_field(ptr, end);
 
     if (ZReadAscii32(ptr, end-ptr, &temp) == ZERR_BADFIELD)
 	BAD_PACKET;
     numfields = temp;
-    next_field (ptr);
+    ptr = next_field(ptr, end);
 
     /*XXX 3 */
     numfields -= 2; /* numfields, version, and checksum */
@@ -139,151 +100,153 @@ Code_t ZParseNotice(buffer, len, notice)
 	return ZERR_BADPKT;
     }
 
-    if (numfields) {
+    if (numfields && ptr < end) {
 	if (ZReadAscii32(ptr, end-ptr, &temp) == ZERR_BADFIELD)
 	    BAD_PACKET;
 	notice->z_kind = (ZNotice_Kind_t)temp;
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	BAD_PACKET;
 	
-    if (numfields) {
+    if (numfields && ptr < end) {
 	if (ZReadAscii(ptr, end-ptr, (unsigned char *)&notice->z_uid,
 		       sizeof(ZUnique_Id_t)) == ZERR_BADFIELD)
 	    BAD_PACKET;
 	notice->z_time.tv_sec = ntohl((u_long) notice->z_uid.tv.tv_sec);
 	notice->z_time.tv_usec = ntohl((u_long) notice->z_uid.tv.tv_usec);
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	BAD_PACKET;
 	
-    if (numfields) {
+    if (numfields && ptr < end) {
 	if (ZReadAscii16(ptr, end-ptr, &notice->z_port) == ZERR_BADFIELD)
 	    BAD_PACKET;
 	notice->z_port = htons(notice->z_port);
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	BAD_PACKET;
 
-    if (numfields) {
+    if (numfields && ptr < end) {
 	if (ZReadAscii32(ptr, end-ptr, &temp) == ZERR_BADFIELD)
 	    BAD_PACKET;
 	notice->z_auth = temp;
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	BAD_PACKET;
     notice->z_checked_auth = ZAUTH_UNSET;
 	
-    if (numfields) {
+    if (numfields && ptr < end) {
 	if (ZReadAscii32(ptr, end-ptr, &temp) == ZERR_BADFIELD)
 	    BAD_PACKET;
 	notice->z_authent_len = temp;
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	BAD_PACKET;
 
-    if (numfields) {
+    if (numfields && ptr < end) {
 	notice->z_ascii_authent = ptr;
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	BAD_PACKET;
 
-    if (numfields) {
+    if (numfields && ptr < end) {
 	notice->z_class = ptr;
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	notice->z_class = "";
 	
-    if (numfields) {
+    if (numfields && ptr < end) {
 	notice->z_class_inst = ptr;
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	notice->z_class_inst = "";
 
-    if (numfields) {
+    if (numfields && ptr < end) {
 	notice->z_opcode = ptr;
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	notice->z_opcode = "";
 
-    if (numfields) {
+    if (numfields && ptr < end) {
 	notice->z_sender = ptr;
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	notice->z_sender = "";
 
-    if (numfields) {
+    if (numfields && ptr < end) {
 	notice->z_recipient = ptr;
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	notice->z_recipient = "";
 
-    if (numfields) {
+    if (numfields && ptr < end) {
 	notice->z_default_format = ptr;
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	notice->z_default_format = "";
 	
-/*XXX*/
     if (ZReadAscii32(ptr, end-ptr, &temp) == ZERR_BADFIELD)
 	BAD_PACKET;
     notice->z_checksum = temp;
     numfields--;
-    next_field (ptr);
+    ptr = next_field(ptr, end);
 
-    if (numfields) {
+    if (numfields && ptr < end) {
 	notice->z_multinotice = ptr;
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	notice->z_multinotice = "";
 
-    if (numfields) {
+    if (numfields && ptr < end) {
 	if (ZReadAscii(ptr, end-ptr, (unsigned char *)&notice->z_multiuid,
 		       sizeof(ZUnique_Id_t)) == ZERR_BADFIELD)
 	    BAD_PACKET;
 	notice->z_time.tv_sec = ntohl((u_long) notice->z_multiuid.tv.tv_sec);
 	notice->z_time.tv_usec = ntohl((u_long) notice->z_multiuid.tv.tv_usec);
 	numfields--;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     else
 	notice->z_multiuid = notice->z_uid;
 
-    for (i=0;i<Z_MAXOTHERFIELDS && numfields;i++,numfields--) {
+    for (i=0;ptr < end && i<Z_MAXOTHERFIELDS && numfields;i++,numfields--) {
 	notice->z_other_fields[i] = ptr;
-	next_field (ptr);
+	ptr = next_field(ptr, end);
     }
     notice->z_num_other_fields = i;
     
-    for (i=0;i<numfields;i++)
-	next_field (ptr);
-	
+    for (i=0;ptr < end && numfields;numfields--)
+	ptr = next_field(ptr, end);
+
+    if (numfields || *(ptr - 1) != '\0')
+	BAD_PACKET;
+
     notice->z_message = (caddr_t) ptr;
     notice->z_message_len = len-(ptr-buffer);
 
diff --git a/lib/ZReadAscii.c b/lib/ZReadAscii.c
index f826bc7..53037cb 100644
--- a/lib/ZReadAscii.c
+++ b/lib/ZReadAscii.c
@@ -46,17 +46,16 @@ Code_t ZReadAscii(ptr, len, field, num)
     register unsigned int temp;
 
     for (i=0;i<num;i++) {
-	if (*ptr == ' ') {
+	if (len >= 1 && *ptr == ' ') {
 	    ptr++;
-	    if (--len < 0)
-		return ZERR_BADFIELD;
-	} 
-	if (ptr[0] == '0' && ptr[1] == 'x') {
+	    len--;
+	}
+	if (len >= 2 && ptr[0] == '0' && ptr[1] == 'x') {
 	    ptr += 2;
 	    len -= 2;
-	    if (len < 0)
-		return ZERR_BADFIELD;
-	} 
+	}
+	if (len < 2)
+	    return ZERR_BADFIELD;
 	c1 = Z_cnvt_xtoi(ptr[0]);
 	if (c1 < 0)
 		return ZERR_BADFIELD;
@@ -67,8 +66,6 @@ Code_t ZReadAscii(ptr, len, field, num)
 	field[i] = hexbyte;
 	ptr += 2;
 	len -= 2;
-	if (len < 0)
-	    return ZERR_BADFIELD;
     }
 
     return *ptr ? ZERR_BADFIELD : ZERR_NONE;
diff --git a/lib/Zinternal.c b/lib/Zinternal.c
index 9fb9a3f..1a3fcec 100644
--- a/lib/Zinternal.c
+++ b/lib/Zinternal.c
@@ -471,6 +471,11 @@ Code_t Z_AddNoticeToEntry(qptr, notice, part)
     struct _Z_Hole *hole, *lasthole;
     struct timeval tv;
 
+    /* Bounds check. */
+    if (part < 0 || notice->z_message_len < 0 || part > qptr->msg_len
+	|| notice->z_message_len > qptr->msg_len - part)
+      return (ZERR_NONE);
+
     /* Incorporate this notice's checked authentication. */
     if (notice->z_checked_auth == ZAUTH_FAILED)
 	qptr->auth = ZAUTH_FAILED;