
OS X 10.11 buffer overflow with deep filesystem hierarchy - ivank
https://cxsecurity.com/issue/WLB-2015100149?
======
userbinator
The article references
[http://www.opensource.apple.com/source/Libc/Libc-1044.40.1/g...](http://www.opensource.apple.com/source/Libc/Libc-1044.40.1/gen/fts.c)
but there's nothing that particularly stands out as being the bug at a quick
glance and it _is_ designed to handle arbitrarily deep hierarchies;
nonetheless this looks like an off-by-one to me (in fts_alloc):

    
    
        len = sizeof(FTSENT) + namelen;
        if (!ISSET(FTS_NOSTAT))
            len += sizeof(struct stat) + ALIGNBYTES;
        if ((p = malloc(len)) == NULL)
            return (NULL);
    
        /* Copy the name plus the trailing NULL. */
        memmove(p->fts_name, name, namelen + 1);
    

It's a pretty minor bug, since I'd bet the chances of it causing a major
exploitable vulnerability are quite low. A theoretical attacker can only
control somewhat coarsely where a single byte with the value 0 can be written,
and this is in userspace. Most of the time it probably lands in unused
padding, and otherwise in an invalid address which causes a segfault.

~~~
pixelbeat
There have been a couple of recent changes to the latest GNU version of that
related to flexible array members:

[https://github.com/coreutils/gnulib/blame/master/lib/fts.c#L...](https://github.com/coreutils/gnulib/blame/master/lib/fts.c#L1901)

[https://github.com/coreutils/gnulib/commit/49078a78](https://github.com/coreutils/gnulib/commit/49078a78)

~~~
just_curioussss
This part is also suspicious:

 _p- >fts_statp = (struct stat _)ALIGN(p->fts_name + namelen + 2);*

Unless ALIGN aligns the pointer towards the lower address( which would be
weird ), that +2 might get out of bounds depending on how many bytes is
ALIGNBYTES.

Please correct me if I'm wrong.

Edit:

This is the header that defines the struct FTSENT:

[https://opensource.apple.com/source/Libc/Libc-1044.40.1/incl...](https://opensource.apple.com/source/Libc/Libc-1044.40.1/include/fts.h)

~~~
userbinator
ALIGNBYTES is probably 3 or 7, depending on what ALIGN does, but the +2 seems
like a bug to me. I think the intended layout is [FTSENT][name][0][optional
[possible alignment padding][struct stat]] which means that +2 should really
be a +1. The total memory allocated should be

    
    
        sizeof(FTSENT) + namelen + 1 + (padding + sizeof(struct stat))
    

(I Googled 'site:opensource.apple.com "#define ALIGNBYTES" inurl:.h' and got
my query rewritten without the quotes and the dots in the domain name. No, I
did _NOT_ mean to search for anything else. Then I browsed to the 2nd page and
got the "we detected suspicious activity" CAPTCHA. WTF?)

~~~
just_curioussss
I found a couple different definitions:

[https://opensource.apple.com/source/OpenSSH/OpenSSH-95/opens...](https://opensource.apple.com/source/OpenSSH/OpenSSH-95/openssh/defines.h)

[https://opensource.apple.com/source/xnu/xnu-792.13.8/bsd/ppc...](https://opensource.apple.com/source/xnu/xnu-792.13.8/bsd/ppc/param.h)

[https://opensource.apple.com/source/sendmail/sendmail-32/sen...](https://opensource.apple.com/source/sendmail/sendmail-32/sendmail/libsm/local.h?txt)

They align toward the higher address.

~~~
anonbanker
so the +2 is indeed out of bounds?

~~~
just_curioussss
Depending on the value of _namelen_ , and the padding at the end of the struct
FTSENT, struct _p- >fts_statp_ will occupy one byte out of bounds of the
allocated memory.

The comment made in the code is incorrect:

 _Since the fts_name field is declared to be of size 1, the fts_name pointer
is namelen + 2 before the first possible address of the stat structure._

namelen + 1 is the first possible address for the stat structure.

~~~
just_curioussss
Note, this refers to:
[https://opensource.apple.com/source/Libc/Libc-1044.40.1/gen/...](https://opensource.apple.com/source/Libc/Libc-1044.40.1/gen/fts.c)

~~~
just_curioussss
No one will ever read this, but I just wanted to point out that the whole
allocation is simply incorrect.

Even with just _namelen+1_ you can still get undefined behavior. This is
because if namelen is shorter than the padding of the object struct FTSENT,
the beginning of the next object struct stat, will overlap with the previous
object.

The correct solution is to make sure that the next object begins after the
first one, and still remains inside of the allocated block. This is a good
example, why the struct hack just isn't worth it, and it itself is arguably
undefined behavior.

To allocate it all, remove the struct hack and simply call the malloc three
times, once for each struct and then for the string. If one allocation is
required, then allocate enough memory for all three objects separated by
enough alignment padding. Doing this will allocate a couple of bytes more
which is a couple percent overall, but at least your code will be correct.
Since they don't pack the struct thus loosing bytes for internal padding
anyway, I can't understand why the usage of the struct hack.

------
reilly3000
So in theory if you wanted to take down a web server that allows for creating
files in a directory structure you just have to nest less than 1k of them to
start wreaking havoc? Yikes if so.

~~~
trengrj
An OS X web server... As another comment mentioned OS X is hindered by the
fact they refuse to use GPL 3 code [http://meta.ath0.com/2012/02/05/apples-
great-gpl-purge/](http://meta.ath0.com/2012/02/05/apples-great-gpl-purge/).
This results in their userspace code being out of date (i.e. they still use
bash version 3.2) and having bugs like these.

~~~
the_mitsuhiko
> As another comment mentioned OS X is hindered by the fact they refuse to use
> GPL 3 code

They do not refuse, they cannot include it from a legal point of view.

~~~
hedwall
What is the difference between GPL2 and GPL3 that makes this impossible from a
legal POV?

~~~
Someone
I don't think anybody knows whether there is a real difference, but the
increased strictness of GPL3 such as its anti-TIVOization
([https://en.m.wikipedia.org/wiki/Tivoization](https://en.m.wikipedia.org/wiki/Tivoization))
clauses certainly make a lot of lawyers concerned.

Where exactly the border lies between and not linking is a grey area, legally,
that hasn't become clearer with GPL3.

It wouldn't surprise me to see bash go from the standard install, too. Apple
is slowly removing all GPL code from its OS.

~~~
autoreleasepool
I hope they make ZSH the standard shell. It's similar enough to bash to not
disrupt most workflows and Apple has been able to update it due to its
permissive license.

I use ZSH as my main shell on OS X. I find it superior to bash in almost every
way (especially with oh-my-zsh) except in ubiquity. Even so, I have no problem
using bash if that's all that's available, the two shells are fairly
compatible.

~~~
mitchty
It was the primary shell in 10.2 or 10.3 after tcsh or csh (its been a while I
can't remember well).

------
dhess
I reported this bug to Apple back in September, but as we know, filing Radars
is practically pointless and as of today it still hasn't even been read.

[https://openradar.appspot.com/22671534](https://openradar.appspot.com/22671534)

------
thesorrow
Seems like there's a lot more vuln with OSX in the last few releases. I'm
still using OSX for basic stuff like browsing but I rely on a linux VM to do
the serious stuff. I know that my vm can be compromised if the host system is
vulnerable but I feel safer.

~~~
acdha
Have you considered how much of that might be due to Apple's takeover of the
consumer space? There were fewer people looking for exploits and even fewer
reporters covering them before they became the OS of choice for hundreds of
millions of people.

If you believe Linux is that much better, well, you really ought to follow
your distribution's security announcement list. Nobody in the general
computing space has a track record which entitles them to cast aspersions at
the competition.

------
zwp
Since this doesn't cause a system panic... multiple userland utilities have
MAXPATHLEN problems? Is this reproducible with longer filenames and less
depth? (If not: what cares about depth?)

------
devit
This is because OSX uses a feature-limited (and apparently completely broken
given this report...) BSD-derived userspace instead of the GNU software used
with Linux, which are usually specifically designed to not have fixed limits
of any kind.

~~~
daenney
That's because, license wise, they can't include GPL 3 licensed code, aka the
GNU user space.

Also, this bug does not prove that the whole user space is completely broken.
Many people use it daily without ever hitting these bugs. If you go looking
for issues, you'll find them, just as you'll find them on Linux but in
different places. Not to say this doesn't suck and we shouldn't fix them, but
it's hardly proof of anything.

~~~
devit
Where "can't" means "they don't want to because they want to reserve
themselves the ability to screw their users by shipping devices with OSX or
derivatives installed where the user cannot change the software, which the
GPLv3 is specifically designed to prevent".

Considering handling paths is one of the most fundamental functions of the
Unix tools and C library, having buffer overflows there is quite damning.

~~~
daenney
Regardless of your personal dislike of the vendor, there have been plenty of
buffer overflows and other bugs in coreutils/fileutils. The Mac/BSD user space
is by no means an exception.

~~~
lmm
Which is a real indictment of C, when programs this simple have bugs this
severe.

