Bug 238801 - usbdump -r <file> bad read
Summary: usbdump -r <file> bad read
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: usb (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Hans Petter Selasky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-06-25 12:04 UTC by Andrew Reiter
Modified: 2019-07-03 18:19 UTC (History)
1 user (show)

See Also:


Attachments
crash sample (64 bytes, application/octet-stream)
2019-06-25 12:04 UTC, Andrew Reiter
no flags Details
amend crash sample from AFL to be real (1.14 KB, text/plain)
2019-06-25 12:05 UTC, Andrew Reiter
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Reiter 2019-06-25 12:04:15 UTC
Created attachment 205325 [details]
crash sample

Background: With some spare time, I randomly decided to apply AFL to `usbdump -r <file>`. To get it going, I used a seed file of 10240 bytes from /dev/random and additionally augmented the source to avoid the magic value check and the major/minor checks that one sees in usbdump.c:init_rfile(struct usbcap *). With those changes, AFL under VMWare will find this crash in seconds. From the quick diagnosis, it seems to be a bad memory access and the following is based on June 24 2019 usbdump.c from CURRENT copied into a 12.O-RELEASE tree and built (so one is aware) and print_packets() looking to be the culprit:

[arr@gunther ~]$ gdb /usr/obj/usr/src/amd64.amd64/usr.sbin/usbdump/usbdump.full
GNU gdb (GDB) 8.2.1 [GDB v8.2.1 for FreeBSD]
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-portbld-freebsd12.0".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /usr/obj/usr/src/amd64.amd64/usr.sbin/usbdump/usbdump.full...done.
(gdb) r -r usbdump_read_sample
Starting program: /usr/obj/usr/src/amd64.amd64/usr.sbin/usbdump/usbdump.full -r usbdump_read_sample

Program received signal SIGBUS, Bus error.
strlen (str=0x180011000000e2 <error: Cannot access memory at address 0x180011000000e2>) at /usr/src/lib/libc/string/strlen.c:101
101    		va = (*lp - mask01);
(gdb) bt
#0  strlen (str=0x180011000000e2 <error: Cannot access memory at address 0x180011000000e2>) at /usr/src/lib/libc/string/strlen.c:101
#1  0x0000000800397539 in __vfprintf (fp=<optimized out>, locale=0x8004084e8 <__xlocale_global_locale>, fmt0=<optimized out>,
    ap=<optimized out>) at /usr/src/lib/libc/stdio/vfprintf.c:854
#2  0x0000000800395144 in vfprintf_l (fp=0x8004098e8, locale=0x8004084e8 <__xlocale_global_locale>,
    fmt0=0x200b12 "%.*s.%06ld usbus%d.%d %s-%s-EP=%08x,SPD=%s,NFR=%d,SLEN=%d,IVAL=%d%s%s\n", ap=0x7fffffffe720)
    at /usr/src/lib/libc/stdio/vfprintf.c:285
#3  0x00000008003e21c4 in printf (fmt=0x1 <error: Cannot access memory at address 0x1>) at /usr/src/lib/libc/stdio/printf.c:57
#4  0x0000000000203432 in print_apacket (ptr=<optimized out>, ptr_len=<optimized out>, hdr=<optimized out>)
    at /usr/src/usr.sbin/usbdump/usbdump.c:495
#5  print_packets (data=0x800644000 "\f;2\331\366\067\335\233\225\377\262!\226\237\066d\001\245SSSS", datalen=<optimized out>)
    at /usr/src/usr.sbin/usbdump/usbdump.c:631
#6  0x0000000000203104 in read_file (p=<optimized out>) at /usr/src/usr.sbin/usbdump/usbdump.c:676
#7  0x0000000000202e1d in main (argc=3, argv=0x7fffffffeab8) at /usr/src/usr.sbin/usbdump/usbdump.c:888
(gdb) up 5
#5  print_packets (data=0x800644000 "\f;2\331\366\067\335\233\225\377\262!\226\237\066d\001\245SSSS", datalen=<optimized out>)
    at /usr/src/usr.sbin/usbdump/usbdump.c:631
631    				print_apacket(&temp, ptr +
(gdb)  list 605
600    			if (next <= ptr)
601    				err(EXIT_FAILURE, "Invalid length");
602    		}
603    	}
604
605    	static void
606    	print_packets(uint8_t *data, const int datalen)
607    	{
608    		struct header_32 temp;
609    		uint8_t *ptr;
(gdb)
610    		uint8_t *next;
611
612    		for (ptr = data; ptr < (data + datalen); ptr = next) {
613
614    			const struct header_32 *hdr32;
615
616    			hdr32 = (const struct header_32 *)ptr;
617
618    			temp.ts_sec = le32toh(hdr32->ts_sec);
619    			temp.ts_usec = le32toh(hdr32->ts_usec);
(gdb)
620    			temp.caplen = le32toh(hdr32->caplen);
621    			temp.datalen = le32toh(hdr32->datalen);
622    			temp.hdrlen = hdr32->hdrlen;
623    			temp.align = hdr32->align;
624
625    			next = ptr + roundup2(temp.hdrlen + temp.caplen, temp.align);
626
627    			if (next <= ptr)
628    				err(EXIT_FAILURE, "Invalid length");
629
(gdb)
630    			if (verbose >= 0 || r_arg != NULL || b_arg != NULL) {
631    				print_apacket(&temp, ptr +
632    				    temp.hdrlen, temp.caplen);
633    			}
634    			pkt_captured++;
635    		}
636    	}


To fix the crash input to be realistic, I set the magic value, major value to 0, and minor value (of 3, versus 2) for the struct usbcap_filehdr. I only tested with minor value set to 3 and not 2, but it would seem, regardless, to likely want to adjust how that parsing is done. Attached is the input sample.

I have not worked up a patch, but mostly because I did this in some spare cycles and I do not really know this code so well to produce a solution. :( I know that is not very helpful, but figured I should at least file the bug.
Comment 1 Andrew Reiter 2019-06-25 12:05:54 UTC
Created attachment 205326 [details]
amend crash sample from AFL to be real

Attaching amend.c which I used to modify the crash input generated by AFL on the augmented usbdump.c to be one that works on un-modified usbdump.c.
Comment 2 commit-hook freebsd_committer 2019-06-25 13:15:40 UTC
A commit references this bug:

Author: hselasky
Date: Tue Jun 25 13:15:30 UTC 2019
New revision: 349370
URL: https://svnweb.freebsd.org/changeset/base/349370

Log:
  Fix parsing of corrupt data in usbdump(8). Check that the transfer
  type array lookup is within bounds to avoid segfault.

  PR:		238801
  MFC after:	3 days
  Sponsored by:	Mellanox Technologies

Changes:
  head/usr.sbin/usbdump/usbdump.c
Comment 3 Hans Petter Selasky freebsd_committer 2019-06-25 13:16:57 UTC
Thank you for the crash file.

Let me know if you find more such issues.

I don't consider this issue to be harmful in any way.

--HPS
Comment 4 Andrew Reiter 2019-06-25 13:22:37 UTC
That was quick :D Thanks! (and agree on the not being harmful point).
Best,
Andrew
Comment 5 commit-hook freebsd_committer 2019-07-03 18:18:51 UTC
A commit references this bug:

Author: hselasky
Date: Wed Jul  3 18:18:05 UTC 2019
New revision: 349664
URL: https://svnweb.freebsd.org/changeset/base/349664

Log:
  MFC r349370:
  Fix parsing of corrupt data in usbdump(8). Check that the transfer
  type array lookup is within bounds to avoid segfault.

  PR:		238801
  Sponsored by:	Mellanox Technologies

Changes:
_U  stable/12/
  stable/12/usr.sbin/usbdump/usbdump.c
Comment 6 commit-hook freebsd_committer 2019-07-03 18:18:53 UTC
A commit references this bug:

Author: hselasky
Date: Wed Jul  3 18:18:43 UTC 2019
New revision: 349665
URL: https://svnweb.freebsd.org/changeset/base/349665

Log:
  MFC r349370:
  Fix parsing of corrupt data in usbdump(8). Check that the transfer
  type array lookup is within bounds to avoid segfault.

  PR:		238801
  Sponsored by:	Mellanox Technologies

Changes:
_U  stable/11/
  stable/11/usr.sbin/usbdump/usbdump.c
Comment 7 commit-hook freebsd_committer 2019-07-03 18:19:55 UTC
A commit references this bug:

Author: hselasky
Date: Wed Jul  3 18:19:29 UTC 2019
New revision: 349666
URL: https://svnweb.freebsd.org/changeset/base/349666

Log:
  MFC r349370:
  Fix parsing of corrupt data in usbdump(8). Check that the transfer
  type array lookup is within bounds to avoid segfault.

  PR:		238801
  Sponsored by:	Mellanox Technologies

Changes:
_U  stable/10/
  stable/10/usr.sbin/usbdump/usbdump.c