Re: [Audacity-devel] Please drop PATH_MAX limitation

[Richard A. <ri...@au...>]

On Tue, 09 Apr 2013 00:06:36 +0100
Martyn Shaw <mar...@gm...> wrote:
> As you say, not every system has this limit, but some do and are open 
> to vulnerabilities if we ignore it.  Your quote on bug #633 "...you 
> need to either use a different implementation that does not
> rely on the length of a string..." and I think that would have to be 
> conditional for that particular OS, so we don't destabilise others.
> 
> Presumably you are looking to support GNU Hurd (which I hadn't heard 
> of before).
There is also an issue on Linux if you are not very careful, because
file / folder names longer than MAX_PATH are perfectly possible and
legal (the constant is used for certain kernel interfaces, but does not
apply to userspace).

When I have time I am intending to construct a script to test this.

> If somebody constructed a 1GB aup file with a very long 
> file name in it, your code would just keep allocating memory.  What
> if it were bigger?  1TB?  There should be some sanity check?

A good point. There is also another sort of problem which your patch
could produce, which is that you are adding a function which returns
malloc'd memory. This is inherently a risky programming pattern, one
which is prone to causing memory leaks.

At a minimum the function needs documenting to point out that the
caller is responsible for freeing the returned string. I would much
prefer a wxreadlink() function which encapsulates the malloc'd buffer
and returns a wxString object, which is then safe to handle however.

> This
> http://audacity.238276.n2.nabble.com/Fwd-RE-Is-the-latest-BETA-version-of-Audacity-still-vulnerable-to-these-buffer-overflow-vulns-td5883514.html
> 
> may be relevant to the current situation, I think.

That was one stage worse if I remember rightly - there was a fixed size
buffer which was possible to overflow, thus overwriting other memory
and crashing the application. The fix there uses OO string objects,
which prevents the stack smash, but does not prevent excessive memory
allocation with a corrupt file.

Richard