Subject: bin/28157: escape sequence of hexdump(1) is broken (SIGSEGV at worst)
To: None <gnats-admin@netbsd.org, netbsd-bugs@netbsd.org>
From: KAMADA Ken'ichi <kamada@nanohz.org>
List: netbsd-bugs
Date: 11/10/2004 10:02:00
>Number:         28157
>Category:       bin
>Synopsis:       escape sequence of hexdump(1) is broken (SIGSEGV at worst)
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Nov 10 10:02:00 +0000 2004
>Originator:     KAMADA Ken'ichi
>Release:        NetBSD 2.99.10
>Organization:
>Environment:
System: NetBSD mitana.nanohz.org 2.99.10 NetBSD 2.99.10 (MITANA) #33: Fri Oct 22 16:37:36 JST 2004 ken@mitana.nanohz.org:/usr/src/sys/arch/i386/compile/MITANA i386
Architecture: i386
Machine: i386
>Description:
The way how hexdump(1) parses escape sequences has some bugs.
It shows up when an escape sequence is used as the non-last character
of a format string.

>How-To-Repeat:
% hexdump -e '8/1 "\\x%02x" "\n"' /path/to/some/data
hexdump: %
: bad conversion character 

% hexdump -e '1/1 "\\x%02x"' /path/to/some/data
Segmentation fault (core dumped)

% hexdump -e '1/1 "\nx"' /path/to/some/data
--> many 'n's are output. (instead of 'x's)

>Fix:
There may be similar bugs, but the following patch fixes this perticular
problem.

Built-in formats should not have been broken with this patch...
# simply checked as follows.
#   for opt in b c C d o x; do
#       hexdump -$opt /usr/bin/hexdump > t1
#       ./hexdump.new -$opt /usr/bin/hexdump > t2
#       cmp t1 t2
#   done

Index: parse.c
===================================================================
RCS file: /cvsroot/src/usr.bin/hexdump/parse.c,v
retrieving revision 1.17
diff -u -r1.17 parse.c
--- parse.c	27 Oct 2003 00:12:43 -0000	1.17
+++ parse.c	10 Nov 2004 09:47:12 -0000
@@ -254,10 +254,10 @@
 			if (fu->bcnt) {
 				sokay = USEBCNT;
 				/* Skip to conversion character. */
-				for (++p1; strchr(spec, *p1); ++p1);
+				for (++p1; *p1 && strchr(spec, *p1); ++p1);
 			} else {
 				/* Skip any special chars, field width. */
-				while (strchr(spec + 1, *++p1));
+				while (*++p1 && strchr(spec + 1, *p1));
 				if (*p1 == '.' &&
 				    isdigit((unsigned char)*++p1)) {
 					sokay = USEPREC;
@@ -268,7 +268,7 @@
 					sokay = NOTOKAY;
 			}
 
-			p2 = p1 + 1;		/* Set end pointer. */
+			p2 = *p1 ? p1 + 1 : p1;	/* Set end pointer. */
 			cs[0] = *p1;		/* Set conversion string. */
 			cs[1] = '\0';
 
@@ -433,6 +433,8 @@
 		    !(fu->flags&F_SETREP) && fu->bcnt)
 			fu->reps += (blocksize - fs->bcnt) / fu->bcnt;
 		if (fu->reps > 1) {
+			if (!fu->nextpr)
+				break;
 			for (pr = fu->nextpr;; pr = pr->nextpr)
 				if (!pr->nextpr)
 					break;
@@ -466,6 +468,10 @@
 		}
 		if (*p1 == '\\')
 			switch(*++p1) {
+			case '\0':
+				*p2 = '\\';
+				*++p2 = '\0';
+				return;		/* incomplete escape sequence */
 			case 'a':
 			     /* *p2 = '\a'; */
 				*p2 = '\007';
@@ -492,6 +498,8 @@
 				*p2 = *p1;
 				break;
 			}
+		else
+			*p2 = *p1;
 	}
 }