segfault bb_make_directory + dirname with musl

Tito farmatito at tiscali.it
Tue Dec 6 21:32:04 UTC 2016



On 11/30/2016 11:52 PM, Denys Vlasenko wrote:
> On Wed, Nov 30, 2016 at 3:46 AM, Daniel Sabogal <dsabogalcc at gmail.com> wrote:
>> The following commands cause busybox to segfault on musl-based systems.
>>
>> $ install -D a /
>> $ install -D a /b
>> $ install -D a /b/
>>
>> This happens because the code in
>>
>> https://git.busybox.net/busybox/tree/coreutils/install.c?h=1_25_1#n196
>>
>> passes the result of dirname() to bb_make_directory() which modifies its
>> contents. For paths of the above forms, musl's dirname returns a string
>> literal "/" which shouldn't be modified.
>>
>> See http://git.musl-libc.org/cgit/musl/tree/src/misc/dirname.c
>>
>> There are a few other occurrences of the code shown above, but I've not
>> checked to see if they could be made to segfault.
>
> Does this fix the problem?
>
>                         /* Bypass leading non-'/'s and then subsequent '/'s */
>                         while (*s) {
>                                 if (*s == '/') {
>                                         do {
>                                                 ++s;
>                                         } while (*s == '/');
>                                         c = *s; /* Save the current char */
> ====added line==>                       if (c)
>                                                 *s = '\0'; /* and
> replace it with nul */
>                                         break;
> _______________________________________________
Hi,
don't know if this could be useful, but reading this thread inspired
me to write dirname and basename replacement functions for busybox
that return a malloced string that could be modified by the caller.
The functions seem to work nice as standalone program and both
pass the tests as in the man 3 basename examples and a few more
added by me.

/*  Man 3 BASENAME examples (taken from SUSv2)
     path       dirname   basename
        /usr/lib   /usr      lib
        /usr/      /         usr
        usr        .         usr
        /          /         /
        .          .         .
        ..         .         ..
      Added examples:
        usr/lib   usr       lib
        usr/lib/  usr       lib
        usr/       .        usr
        /usr       .        usr
        /a/b/c     /a/b      c
        /a/b/c/    /a/b      c
        a/b/c      a/b       c
        a/b/c/     a/b       c
        //         /         /
        ///        /         /
        ////       /         /
        '/a/b/ '   /a/b     ' '
*/

The functions look like this:

char* bb_basename_malloc(const char *name)
{
	const char *p1 = NULL;
	const char *p2 = NULL;
	const char *s = name;
	char *last = last_char_is(s, '/');

	while (*s) {
		if (*s == '/') {
			p2 = p1;
			p1 = s;
		}
		s++;
	}
	if (last) {
		if(p2)
			name = p2 + 1;
	} else if (p1) {
		name = p1 + 1;
	}
	return xstrndup(name, strlen(name) - (last && last != name));
}

char* bb_dirname_malloc(const char *name)
{
	int len = 0;
	const char *p1 = NULL;
	const char *p2 = NULL;
	const char *s = name;
	
	while (*s) {
		if (*s == '/' && ((s[1] && s[1] != '/') || s == name)) {
			p2 = p1;
			p1 = s;
		}
		s++;
	}
	if (!p2 && !p1)
		return xstrdup(".");
	if (p1 == name && !p2)
		return xstrdup("/");
	if (p1)
		len  =  strlen(p1);
	return xstrndup(name, strlen(name) - len);
}

Attached you will find the standalone version to test the functions.
Hints, critics, improvements are welcome.

Ciao,
Tito
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bb_dirbasename_malloc.c
Type: text/x-csrc
Size: 9964 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20161206/ae4e2d29/attachment.c>


More information about the busybox mailing list