[Bug 13131] New: archival/libarchive/get_header_tar.c can pass a NULL ptr to last_char_is()

[bugzilla at busybox.net]

https://bugs.busybox.net/show_bug.cgi?id=13131

            Bug ID: 13131
           Summary: archival/libarchive/get_header_tar.c can pass a NULL
                    ptr to last_char_is()
           Product: Busybox
           Version: 1.33.x
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: major
          Priority: P5
         Component: Other
          Assignee: unassigned at busybox.net
          Reporter: ninuje at gmail.com
                CC: busybox-cvs at busybox.net
  Target Milestone: ---

On master, the most recent tweak to last_char_is
(https://git.busybox.net/busybox/commit/?id=79a4032eefe405a1e7d4a644614fcc4a07b98d88)
removed the guard against NULL pointers.

Unfortunately, at least one caller can (fairly frequently, apparently) pass a
NULL pointer: the tar applet (via archival/libarchive/get_header_tar.c:355).

This can be easily reproduced by building current master (defconfig), and
trying to parse pretty much any tarball you can find (say, busybox's 1.32.0
sources):

-rw-r--r-- root/root       116 2018-12-05 15:44:34
busybox-1.32.0/shell/ash_test/ash-vars/param_expand_len.right

Program received signal SIGSEGV, Segmentation fault.
last_char_is (s=0x0, c=47) at libbb/last_char_is.c:14
14              if (!s[0])
(gdb) bt full
#0  last_char_is (s=0x0, c=47) at libbb/last_char_is.c:14
No locals.
#1  0x00005555555fc54f in get_header_tar (archive_handle=0x55555565a2a0) at
archival/libarchive/get_header_tar.c:355
        file_header = 0x55555565a360
        tar = {name =
"busybox-1.32.0/shell/ash_test/ash-vars/var-do-not-quote-backslashes-in-parameter-expansions-outside-",
mode = "0000755", uid = "\000\060\060\060\060\060\060", gid =
"\000\060\060\060\060\060\060", size =
"\000\060\060\060\060\060\060\060\060\063\064", mtime =
"\000\063\064\060\061\067\066\062\061\062\062", 
          chksum = "\000\063\062\062\065\064\000 ", typeflag = 0 '\000',
linkname = '\000' <repeats 99 times>, magic = "ustar  ", uname = "root", '\000'
<repeats 27 times>, gname = "root", '\000' <repeats 27 times>, devmajor =
"\000\000\000\000\000\000\000", devminor = "\000\000\000\000\000\000\000",
prefix = '\000' <repeats 154 times>, 
          padding = '\000' <repeats 11 times>}
        cp = <optimized out>
        tar_typeflag = 48
        i = <optimized out>
        sum_u = <optimized out>
        sum = <optimized out>
        sum_s = <optimized out>
        parse_names = 1
#2  0x00005555555f35bf in tar_main (argc=<optimized out>, argv=<optimized out>)
at archival/tar.c:1281
        tar_handle = 0x55555565a2a0
        base_dir = 0x0
        tar_filename = 0x7fffffffd734 "busybox-1.32.0.tar.bz2"
        opt = <optimized out>
        verboseFlag = 2
        excludes = 0x0
#3  0x000055555556aae2 in run_applet_no_and_exit (argv=0x7fffffffd270,
name=<optimized out>, applet_no=326) at libbb/appletlib.c:999
        argc = 3
#4  run_applet_and_exit (name=<optimized out>, argv=0x7fffffffd270) at
libbb/appletlib.c:1017
        applet = 326
#5  0x000055555556a8fe in busybox_main (argv=0x7fffffffd270) at
libbb/appletlib.c:960
        a = <optimized out>
        col = <optimized out>
        output_width = <optimized out>
        len2 = <optimized out>
        n = <optimized out>
        i = <optimized out>
        a = <optimized out>
        v = <optimized out>
        use_symbolic_links = <optimized out>
        busybox = <optimized out>
#6  run_applet_and_exit (name=<optimized out>, argv=<optimized out>) at
libbb/appletlib.c:1010
No locals.
#7  0x000055555556b6f0 in main (argc=<optimized out>, argv=0x7fffffffd268) at
libbb/appletlib.c:1151
No locals.


Restoring the NULL guard fixes this:

diff --git a/libbb/last_char_is.c b/libbb/last_char_is.c
index fba05f974..eca612a6e 100644
--- a/libbb/last_char_is.c
+++ b/libbb/last_char_is.c
@@ -11,7 +11,7 @@
 /* Find out if the last character of a string matches the one given */
 char* FAST_FUNC last_char_is(const char *s, int c)
 {
-       if (!s[0])
+       if (!s || !s[0])
                return NULL;
        while (s[1])
                s++;

-- 
You are receiving this mail because:
You are on the CC list for the bug.