From: Guy Harris <guy@alum.mit.edu>
Date: Mon, 20 May 2019 07:25:10 +0000 (-0700)
Subject: Do more bounds checking in unistr().
X-Git-Tag: tcpdump-4.99-bp~773
X-Git-Url: https://git.tcpdump.org/tcpdump/commitdiff_plain/daa343d8e215a6704ee37399a77a70133df51e01?ds=inline

Do more bounds checking in unistr().

If we quit because we see a NUL, make sure we have all the remaining
data claimed to be in the string, so that our caller doesn't add a
ridiculously large number to the current packet pointer and overflow it.

Keep processing packets even if we see more than MAX_UNISTR_SIZE
characters to print - just don't add them to the buffer - so that we
check the presence of every byte in the string.

Also, print the string even if it was truncated, *but* quit processing
after that.  Print a newline if smb_fdata1() indicates a truncation and
if there's a newline in the format string after the item being printed,
so the newline is printed no matter what.
---

diff --git a/smbutil.c b/smbutil.c
index 0acb553f..69d29c66 100644
--- a/smbutil.c
+++ b/smbutil.c
@@ -344,6 +344,7 @@ static int
 unistr(netdissect_options *ndo, char (*buf)[MAX_UNISTR_SIZE+1],
        const u_char *s, uint32_t *len, int is_null_terminated, int use_unicode)
 {
+    u_int c;
     size_t l = 0;
     uint32_t strsize;
     const u_char *sp;
@@ -362,6 +363,7 @@ unistr(netdissect_options *ndo, char (*buf)[MAX_UNISTR_SIZE+1],
     if (is_null_terminated) {
 	/*
 	 * Null-terminated string.
+	 * Find the length, counting the terminating NUL.
 	 */
 	strsize = 0;
 	sp = s;
@@ -394,45 +396,75 @@ unistr(netdissect_options *ndo, char (*buf)[MAX_UNISTR_SIZE+1],
     }
     if (!use_unicode) {
     	while (strsize != 0) {
-          ND_TCHECK_1(s);
-	    if (l >= MAX_UNISTR_SIZE)
-		break;
-	    if (ND_ISPRINT(GET_U_1(s)))
-		(*buf)[l] = GET_U_1(s);
-	    else {
-		if (GET_U_1(s) == 0)
-		    break;
-		(*buf)[l] = '.';
-	    }
-	    l++;
+	    ND_TCHECK_1(s);
+	    c = GET_U_1(s);
 	    s++;
 	    strsize--;
+	    if (c == 0) {
+		/*
+		 * Even counted strings may have embedded null
+		 * terminators, so quit here, and skip past
+		 * the rest of the data.
+		 *
+		 * Make sure, however, that the rest of the data
+		 * is there, so we don't overflow the buffer when
+		 * skipping past it.
+		 */
+		ND_TCHECK_LEN(s, strsize);
+		break;
+	    }
+	    if (l < MAX_UNISTR_SIZE) {
+		if (ND_ISPRINT(c)) {
+		    /* It's a printable ASCII character */
+		    (*buf)[l] = c;
+		} else {
+		    /* It's a non-ASCII character or a non-printable ASCII character */
+		    (*buf)[l] = '.';
+		}
+		l++;
+	    }
 	}
     } else {
-	while (strsize != 0) {
+	while (strsize > 1) {
 	    ND_TCHECK_2(s);
-	    if (l >= MAX_UNISTR_SIZE)
-		break;
-	    if (GET_U_1(s + 1) == 0 && ND_ISPRINT(GET_U_1(s))) {
-		/* It's a printable ASCII character */
-		(*buf)[l] = GET_U_1(s);
-	    } else {
-		/* It's a non-ASCII character or a non-printable ASCII character */
-		if (GET_U_1(s) == 0 && GET_U_1(s + 1) == 0)
-		    break;
-		(*buf)[l] = '.';
-	    }
-	    l++;
+	    c = GET_LE_U_2(s);
 	    s += 2;
-	    if (strsize == 1)
-		break;
 	    strsize -= 2;
+	    if (c == 0) {
+		/*
+		 * Even counted strings may have embedded null
+		 * terminators, so quit here, and skip past
+		 * the rest of the data.
+		 *
+		 * Make sure, however, that the rest of the data
+		 * is there, so we don't overflow the buffer when
+		 * skipping past it.
+		 */
+		ND_TCHECK_LEN(s, strsize);
+		break;
+	    }
+	    if (l < MAX_UNISTR_SIZE) {
+		if (ND_ISPRINT(c)) {
+		    /* It's a printable ASCII character */
+		    (*buf)[l] = c;
+		} else {
+		    /* It's a non-ASCII character or a non-printable ASCII character */
+		    (*buf)[l] = '.';
+		}
+		l++;
+	    }
+	}
+	if (strsize == 1) {
+	    /* We have half of a code point; skip past it */
+	    ND_TCHECK_1(s);
+	    s++;
 	}
     }
     (*buf)[l] = 0;
     return 0;
 
 trunc:
+    (*buf)[l] = 0;
     return -1;
 }
 
@@ -647,11 +679,13 @@ smb_fdata1(netdissect_options *ndo,
 	  {
 	    /*XXX unistr() */
 	    uint32_t len;
+	    int result;
 
 	    len = 0;
-	    if (unistr(ndo, &strbuf, buf, &len, 1, (*fmt == 'R') ? 0 : unicodestr) == -1)
-		goto trunc;
+	    result = unistr(ndo, &strbuf, buf, &len, 1, (*fmt == 'R') ? 0 : unicodestr);
 	    ND_PRINT("%s", strbuf);
+	    if (result == -1)
+		goto trunc;
 	    buf += len;
 	    fmt++;
 	    break;
@@ -660,6 +694,7 @@ smb_fdata1(netdissect_options *ndo,
 	case 'Y':	/* like 'Z', but always ASCII */
 	  {
 	    uint32_t len;
+	    int result;
 
 	    ND_TCHECK_1(buf);
 	    if (GET_U_1(buf) != 4 && GET_U_1(buf) != 2) {
@@ -667,9 +702,10 @@ smb_fdata1(netdissect_options *ndo,
 		return maxbuf;	/* give up */
 	    }
 	    len = 0;
-	    if (unistr(ndo, &strbuf, buf + 1, &len, 1, (*fmt == 'Y') ? 0 : unicodestr) == -1)
-		goto trunc;
+	    result = unistr(ndo, &strbuf, buf + 1, &len, 1, (*fmt == 'Y') ? 0 : unicodestr);
 	    ND_PRINT("%s", strbuf);
+	    if (result == -1)
+		goto trunc;
 	    buf += len + 1;
 	    fmt++;
 	    break;
@@ -697,9 +733,12 @@ smb_fdata1(netdissect_options *ndo,
 	  }
 	case 'C':
 	  {
-	    if (unistr(ndo, &strbuf, buf, &stringlen, 0, unicodestr) == -1)
-		goto trunc;
+	    int result;
+
+	    result = unistr(ndo, &strbuf, buf, &stringlen, 0, unicodestr);
 	    ND_PRINT("%s", strbuf);
+	    if (result == -1)
+		goto trunc;
 	    buf += stringlen;
 	    fmt++;
 	    break;
@@ -872,8 +911,17 @@ smb_fdata(netdissect_options *ndo,
 	    s[p - fmt] = '\0';
 	    fmt = p + 1;
 	    buf = smb_fdata1(ndo, buf, s, maxbuf, unicodestr);
-	    if (buf == NULL)
+	    if (buf == NULL) {
+		/*
+		 * Truncated.
+		 * Is the next character a newline?
+		 * If so, print it before quitting, so we don't
+		 * get stuff in the middle of the line.
+		 */
+		if (*fmt == '\n')
+		    ND_PRINT("\n");
 		return(NULL);
+	    }
 	    break;
 
 	default: