On 10/17/19 4:09 PM, Denys Nykula wrote: >> the tar "no ../ files outside this directory" check is >> false positiving on "./" for some reason, gotta track that down. > > Here's a reproducible toy tar crash, might or might not be connected.
I wasn't seeing a segfault. > git clone https://github.com/landley/toybox > wget http://musl.cc/x86_64-linux-musl-native.tgz > rm -r root.tar toybox/root x86_64-linux-musl-native > (cd toybox; make defconfig root) > toybox/toybox tar xf x86_64-linux-musl-native.tgz > (cd toybox/root/*/*fs; ../../../toybox tar c *)>root.tar > cd x86_64-linux-musl-native &&../toybox/toybox tar xvf ../root.tar > # ... > # tar: can't remove: bin: Is a directory > # tar: can't remove: lib: Is a directory > # tar: 'usr/' not under '/path/to/x86_64-linux-musl-native' Hmmm, the usr->. is confusing it because "usr" _isn't_ under that directory, it _is_ that directory. (It's not a child of it, it's the thing itself.) That's... technically correct? Although it's more a NOP than an error in this corner case. > # Segmentation fault Sigh. Reproduced. It's because sbin is a circular symlink: you have sbin -> usr/sbin and usr -> . so the link points back to itself. This makes xabspath() return null and I'm calling it in a mode where the "x" part doesn't error_exit for that, but didn't handle it myself. :P Huh. What _is_ the right behavior for this? There's _kinda_ some conflicting assumptions in these two filesystems. :) > First two errors are correct, refusing to overwrite an existing dir with > a symlink and moving on, remembering to make a bad exit code later. But > then you hit an existing symlink to a dir. Other implementations replace > the symlink with the dir and keep unpacking files there. Toy tar doesn't > overwrite, says odd message, starts extracting children of a dir that > hasn't been created, and crashes. Full output below. That's not why it crashes, it crashes because it couldn't resolve the absolute path and I called an xfunction() in a mode that didn't error_exit() and then didn't handle the error myself. :) Probably something like: --- a/toys/posix/tar.c +++ b/toys/posix/tar.c @@ -392,7 +392,11 @@ static int dirflush(char *name) // Barf if name not in TT.cwd if (name) { - ss = s = xabspath(name, -1); + if (!(ss = s = xabspath(name, -1))) { + error_msg("'%s' bad symlink", name); + + return 1; + } if (TT.cwd[1] && (!strstart(&ss, TT.cwd) || *ss!='/')) { error_msg("'%s' not under '%s'", name, TT.cwd); free(s); The question is, what behavior do you want? If sbin points to usr/sbin and usr points to . then sbin points to sbin and you _can't_ extract stuff under it. What behavior do you expect here? It looks like the gnu/dammit tar is deleting the usr symlink and replacing it with a directory. I can do that for directories I guess? (Fiddle, fiddle...) Yes, it looks like it's _always_ replacing a symlink with a directory when extract says a directory should be there. Which is a little awkward because the "is this target under here" test comes _before_ it looks at _what_ should go there, so I need to rearrange things a bit when I'm more awake. Rob _______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net