[pacman-dev] [PATCH] Prevent stack overflow on symbolic link access.

Andrew Gregory andrew.gregory.8 at gmail.com
Fri Jun 10 00:38:58 UTC 2016


On 06/09/16 at 10:49pm, Tobias Stoeckmann wrote:
> On Thu, Jun 09, 2016 at 04:25:36PM -0400, Andrew Gregory wrote:
> > SIZE_MAX is the maximum size of individual objects, not the entire
> > addressable space.
> 
> I hope you know that it becomes very very theoretical now, and that there
> is no such system out there...
> 
> Let's assume that there is a system in which size_t is just 16 bit. And
> let's further assume that your pointers are still 32 or 64 bit, giving
> you an address space for many many size_t-sized objects.
> 
> sizeof(size_t) = 2
> sizeof(void *) = 8
> 
> Do you realize that it means that you have to check every single
> occurrence of "size_t len = strlen(x) + 1" for a realistically possible
> overflow?

strlen doesn't count the terminating NUL, so strlen(x) + 1 is at most
the size of the array, which by definition has to fit into a size_t.

> And that pacman doesn't do that just like no other sane
> software, e.g. OpenSSH, because there's no such system out there?
> 
> You would even have to be very careful about pointer arithmetic, because
> it further means that
> 
> sizeof(size_t) = 2
> sizeof(ptrdiff_t) = 8
> 
> So even forget about "size_t len = endptr - baseptr". Granted, that kind
> of code is "buggy" for that reason, but if we go that far, all our
> software is screwed today. And if sizeof(int) eventually turns 8, we
> have a real issue to think about.

Yes, using size_t for pointer arithmetic with arbitrary pointers is
incorrect unless both pointers point to locations within the same
object.  I'm not sure what the existence of poorly written code
elsewhere has to do with pacman.

> If this theoretical thing is of interest, we can further discuss this.
> But I would prefer to get back into reality. Maybe we are, but in that
> case we should boot up a system which has these weird settings. And for
> that, I need a name. It's clearly not OS X, *BSD, or Linux. As pacman
> is a software that targets Linux (as far as I know, it even has *BSD
> support in a few parts), which has a PATH_MAX = 4096 limitation, this
> is of no concern for pacman.
> 
> On the other hand, broken filesystems are out there. I've created an
> image to verify this. In case you need to see the segmentation fault on
> your own, I can prepare a fitting package for it and give you
> instructions on how to trigger it.

I am not rejecting your patch; it is probably better than what we have
now.  I do not like technically incorrect code that relies on
conventions that are not guaranteed to be reliable though.


More information about the pacman-dev mailing list