Bug 13131 - archival/libarchive/get_header_tar.c can pass a NULL ptr to last_char_is()
Summary: archival/libarchive/get_header_tar.c can pass a NULL ptr to last_char_is()
Status: RESOLVED FIXED
Alias: None
Product: Busybox
Classification: Unclassified
Component: Other (show other bugs)
Version: 1.33.x
Hardware: All Linux
: P5 major
Target Milestone: ---
Assignee: unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-08-01 17:55 UTC by NiLuJe
Modified: 2020-10-04 18:24 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description NiLuJe 2020-08-01 17:55:49 UTC
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++;
Comment 1 NiLuJe 2020-10-04 18:24:09 UTC
Fixed the other way around in https://git.busybox.net/busybox/commit/?id=16e82c61d4db91e2e888600cdee7cf656ba4774c ;).