Date: Mon, 16 Jan 2012 21:04:07 +0100 From: Tijl Coosemans <tijl@coosemans.org> To: Eitan Adler <eadler@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r229794 - head/usr.bin/hexdump Message-ID: <201201162104.14841.tijl@coosemans.org> In-Reply-To: <201201072315.q07NFM3v060477@svn.freebsd.org> References: <201201072315.q07NFM3v060477@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--nextPart1980638.SMTQ9Qm2st Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On Sunday 08 January 2012 00:15:22 Eitan Adler wrote: > Author: eadler (ports committer) > Date: Sat Jan 7 23:15:21 2012 > New Revision: 229794 > URL: http://svn.freebsd.org/changeset/base/229794 >=20 > Log: > - Fix how hexdump parses escape strings > From the NetBSD bug: > 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. > =20 > PR: bin/144722 > Submitted by: gcooper > Approved by: rpaulo > Obtained from: NetBSD > MFC after: 1 week >=20 > Modified: > head/usr.bin/hexdump/parse.c >=20 > Modified: head/usr.bin/hexdump/parse.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/usr.bin/hexdump/parse.c Sat Jan 7 22:29:46 2012 (r229793) > +++ head/usr.bin/hexdump/parse.c Sat Jan 7 23:15:21 2012 (r229794) > @@ -255,7 +255,9 @@ rewrite(FS *fs) > sokay =3D NOTOKAY; > } > =20 > - p2 =3D p1 + 1; /* Set end pointer. */ > + p2 =3D *p1 ? p1 + 1 : p1; /* Set end pointer -- make sure > + * that it's non-NUL/-NULL first > + * though. */ > cs[0] =3D *p1; /* Set conversion string. */ > cs[1] =3D '\0'; > =20 > @@ -449,13 +451,21 @@ escape(char *p1) > char *p2; > =20 > /* alphabetic escape sequences have to be done in place */ > - for (p2 =3D p1;; ++p1, ++p2) { > - if (!*p1) { > - *p2 =3D *p1; > - break; > - } > - if (*p1 =3D=3D '\\') > - switch(*++p1) { > + for (p2 =3D p1; *p1; p1++, p2++) { > + /*=20 > + * Let's take a peak at the next item and see whether or not > + * we need to escape the value... > + */ > + if (*p1 =3D=3D '\\') { > + > + p1++; > + > + switch(*p1) { > + /* A standalone `\' */ > + case '\0': > + *p2 =3D '\\'; > + *++p2 =3D '\0'; > + break; This chunk needs to be reworked. This case causes a buffer overflow because p1 points to the end of the string here and is then incremented and dereferenced by the for loop. Also, after the for loop p2 needs to be zero-terminated. Currently, the output has an extra "n" at the beginning of every line: 00000000 2f 2a 2d 0a 20 2a 20 43 6f 70 79 72 69 67 68 74 |/*-. * Copyrig= ht| n00000010 20 28 63 29 20 31 39 39 30 2c 20 31 39 39 33 0a | (c) 1990, 19= 93.| n00000020 20 2a 09 54 68 65 20 52 65 67 65 6e 74 73 20 6f | *.The Regent= s o| > case 'a': > /* *p2 =3D '\a'; */ > *p2 =3D '\007'; > @@ -482,7 +492,12 @@ escape(char *p1) > *p2 =3D *p1; > break; > } > + > + } else > + *p2 =3D *p1; > + > } > + > } > =20 > void >=20 --nextPart1980638.SMTQ9Qm2st Content-Type: application/pgp-signature; name=signature.asc Content-Description: This is a digitally signed message part. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (FreeBSD) iF4EABEIAAYFAk8Ugr4ACgkQfoCS2CCgtivH5gD/f/Jjm2PTrKWFU02jKhwQrG8J gcMWv0OqgS3MMHtjZAwA/2diTyIxPgC42vnZHQVDFNcottAO8NyKcK9ZYPfmvPok =1XQk -----END PGP SIGNATURE----- --nextPart1980638.SMTQ9Qm2st--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201201162104.14841.tijl>