To counter bug #8411, busybox tar no longer allows unsafe symlinks to be created. Unfortunately, it is now so strict that safe symlinks are no longer accepted either. Ironically, the very first failure that I got after upgrading to busybox 1.28 was for a tarball containing along with other files: bin/busybox usr/bin/ar -> ../../bin/busybox At the very least, symlinks to files are safe no matter what they start with. There was no need to block this. It cannot be used to modify any files inappropriately by tar, because a subsequent attempt to extract there would delete the symlink before creating the file. But actually, I think the whole logic for checking unsafe symlinks is unnecessary. It's not the symlink that's unsafe, it's a subsequent attempt to use that symlink to create or overwrite a file where the user shouldn't be able to modify anything. The initial patch handled this for extraction into empty directories by delaying the symlink creation. This is sufficient if the pattern is 1) create temp directory 2) extract into temp directory 3) validate contents of temp directory 4) remove temp directory if contents are invalid. But indeed, this is not enough if multiple untrusted tarballs are extracted into the same directory. I do not know if that is supposed to be safe. It currently isn't with GNU tar, but it may be reasonable to expect that to be safe. One way to handle that would be, on an attempt to extract a/b/c, to require that a and a/b are truly directories, not symlinks. This is doable by using openat(), opening all path components individually with O_NOFOLLOW. This way, if any component is a symlink, an error is raised. If the path components are cached, then for the common case where a large number of files appear in the same directory, it might even reduce path lookups. Would something like this be acceptable, or does this have other security implications that I'm not seeing?
Set EXTRACT_UNSAFE_SYMLINKS=1 for the tar command which extracts something with potentially unsafe symlinks. Act of doing so serves as an explicit statement from the user: "Yes, I know what I'd doing. This tarball is trusted by me".
(In reply to Denys Vlasenko from comment #1) Yes, I'm aware of that, and it's what I'm doing now. I see that as a workaround, not as a fix though: I do actually like the idea of having busybox block attempts to extract outside of the -C directory. It's just that the current check is too strict to be usable for me.
Ouch, this behavior change started bricking embedded systems I work with. Similar to Harald, untarring a new root filesystem containing Busybox onto a target device now fails to unpack all the /sbin/ symlinks (for example the missing "pivot_root -> ../bin/busybox" link was the one that blew up my systems). At this point it is simpler and more robust for me to switch to gnu tar rather than maintain EXTRACT_UNSAFE_SYMLINKS=1 in all the right places.
(In reply to Daniel Goertzen from comment #3) > At this point it is simpler and more robust for me to switch to gnu tar rather than maintain EXTRACT_UNSAFE_SYMLINKS=1 in all the right places. Well then, the following will be happening: > MavProxyUser 2017-07-06 17:30:44 UTC > I can now confirm that this has been exploited "in the wild" to root / jailbreak DJI Mavic, Spark, Inspire2, and Phantom 4 drone series. > Exploit here for posterity > https://github.com/MAVProxyUser/P0VsRedHerring/blob/master/RedHerring.rb#L24 > Thanks for pushing to get this patched. It has festered for a while. It is inescapable fact of life that more secure systems are "less convenient", they would require more steps. It is a balancing act somewhere between having (in this case) tar which will untar anything anywhere, and tar which requires some awful crypto-signed tarballs (this will be very safe, but basically unusable). I am not a security nutjob. I _am_ trying to find a balance, not just bolt everything down to the point where it's securely useless. With EXTRACT_UNSAFE_SYMLINKS=1 thing, I defend even against the case where attacker provides two tarballs, with 1st creating evil symlink, and another using it. GNU tar's solution of delayed symlink creation is defeated by that. *and* unlike a solution of requiring a special option, "tar --extract-unsafe-symlinks", this one does not wreak havoc for all your scripts with tar invocations: envvars are _inherited_. You can just set it in your login environment once, and bingo: all tar invocations will start acting as you want. Propose something better, or how to tweak this solution to be more usable. Specifically, what we have here are symlinks in /sbin to /bin/busybox. Hilariously ad-hoc, but working for us busyboxers solution would be to allow creation of symlinks which end with "bin/busybox" or "/[s]bin/busybox" ;)
Will it help if EXTRACT_UNSAFE_SYMLINKS can be configurable at build time? You can build yourself a busybox binary which does not make this check, just by setting one CONFIG_ option, not with patching sources.
(In reply to Denys Vlasenko from comment #4) > Propose something better, or how to tweak this solution to be more usable. I did have two concrete proposals for something better in my initial comment: 1: Keep the current check, but modify it to allow all symlinks to files to be extracted. OR: 2: Remove the current check, allow all symlinks to be extracted, but add a check that requires all path components during extraction to be real directories, not symlinks. Option 1 should be trivial to implement, no less safe than the current implementation, and sufficient for the use cases mentioned in this report. Option 2 is difficult to implement, but potentially safer since it also protects against symlinks created without the use of tar. An added benefit is that it would never error out when simply creating an archive from directory A and extracting it into directory B, regardless of what symlinks A might contain, so it would have fewer false positives. (In reply to Denys Vlasenko from comment #5) > Will it help if EXTRACT_UNSAFE_SYMLINKS can be configurable at build time? Perhaps, but like I wrote earlier, I do like the idea of this check, so I would prefer to tweak the logic so that it works for more cases.
Bug 10651 - tar: check for unsafe symlinks is overly strict https://bugs.busybox.net/show_bug.cgi?id=10651 | (In reply to Harald van Dijk from comment #6) | 1: Keep the current check, but modify it to allow all symlinks to files to be extracted. (1) What if symlink comes first in the tarball? (2) Is it ok to extract symlink to e.g. "/etc/passwd"? For tar, it _should be ok_ since newly extracted files unlink target filename, then open it with O_CREAT|O_EXCL, thus /etc/passwd can't be overwritten thru this symlink - but it is still feels iffy. Basically, _tar_ will not compromise security - but it may unknowingly create a setup for subsequent writes to the "file" to break it. | OR: | | 2: Remove the current check, allow all symlinks to be extracted, but add a check that requires all path components during extraction to be real directories, not symlinks. | Option 2 is difficult to implement, but potentially safer since it also protects against symlinks created without the use of tar. An added benefit is that it would never error out when simply creating an archive from directory A and extracting it into directory B, regardless of what symlinks A might contain, so it would have fewer false positives. Opt 2 is good. A slight downside is that there is no open flag in Linux which tells open() to fail if any path component is a symlink. O_NOFOLLOW only checks last component. We need to check each one. We will need to lstat() every path component in sequence. Let's also brainstorm option 3: Allow symlinks which (a) start with one or more "../"; (b) never end up on a higher level of directory tree: "../dir/.." is ok, "../../usr/bin/.." is ok, "../file" is not ok, "../../dir/file" is not ok; and (c) they also should not leave tarred tree: symlink "sbin/mkswap" to "../bin/busybox" is ok, but symlink "mkswap" to "../bin/busybox" is not ok (it hops "above" the tree). This sounds a bit complicated, but it can be achieved just by looking at names, no multiple lstat() calls for every open() needed. With only these symlinks allowed, we know that if we untar to an empty directory or to a formerly empty directory with another tarball untarred, any pathname we open, whilst it can contain components which are symlinks, they nevertheless can not allow new opens to "leave" the tree and create files elsewhere. P.S. let's discuss it in the mailing list, other people may have good ideas.
commit a84db18fc71d09e801df0ebca048d82e90b32c6a Author: Denys Vlasenko <vda.linux@googlemail.com> Date: Tue Feb 20 15:57:45 2018 +0100 tar,unzip: postpone creation of symlinks with "suspicious" targets This mostly reverts commit bc9bbeb2b81001e8731cd2ae501c8fccc8d87cc7 "libarchive: do not extract unsafe symlinks unless $EXTRACT_UNSAFE_SYMLINKS=1" Users report that it is somewhat too restrictive. See https://bugs.busybox.net/show_bug.cgi?id=8411 In particular, this interferes with unpacking of busybox-based filesystems with links like "sbin/applet" -> "../bin/busybox". The change is made smaller by deleting ARCHIVE_EXTRACT_QUIET flag - it is unused since 2010, and removing conditionals on it allows commonalizing some error message codes.
Do you guys want 1.28.2 build with this fix?
(In reply to Denys Vlasenko from comment #9) Can't speak for others, but for me there's no urgency, I can just apply your commit until the next release. Thanks though :)
fixed in 1.28.3