bin/86135: Fwd: Latent buffer overflow in getcwd

Bruce Evans bde at zeta.org.au
Thu Sep 15 03:22:03 PDT 2005


On Thu, 15 Sep 2005, Andrey Chernov wrote:

> On Thu, Sep 15, 2005 at 01:27:03PM +1000, Bruce Evans wrote:
>> MAXPATHLEN is not very relevant here -- the size needed is just the size of
>> our buffer, and MAXPATHLEN bytes is neither usually necessary nor always
>
> While it can be so for "up", it is not so for "ep", since it is
> filled by __getcwd() syscall and can't be bigger.
>
> Could you consider MAXPATHLEN for "ep" and 1024 for "up" variant?

The buffer with "ep" is actually "pt" ("ept" is the end of this).  Yes,
it makes no sense to allocate less than {PATH_MAX} bytes for the buffer
with which we make a syscall that might return {PATH_MAX} bytes.

>> - MAXPATHLEN is a misspelling of {PATH_MAX}.
>
> It is BSDsm. getwd(1) refers to MAXPATHLEN too.

imp@ is fixing this BSDism these (except he uses PATH_MAX instead of
{PATH_MAX} so the change is only a style fix) and might not like having
new ones to fix.

BTW, the ERRORS section in getcwd(3) doesn't say that errno is set to
ENAMETOOLONG if the MAXPATHLEN limit is exceeded.

>> - The magic 340 in the above was (1024 - 4) / strlen("../").  Now its
>>   magic is deeper.  340 was wrong even when the initial upsize was known
>>   to be (1024 - 4) since it didn't allow for the NUL terminator or mount
>>   points.  The exact is something like
>>   1 + (initial_upsize - {NAME_MAX} - 1) / strlen("../").
>
> Why ever this magic needed? It is only in comment, not in code.

It is perhaps useful as documentation, but not if it is wrong.  I try not
to put derived magic numbers or their derivation in comments.

Bruce


More information about the freebsd-bugs mailing list