clients/nycli/utils.c: Fix buffer overflow

[sudipm-mukherjee]

format_url() is only assigning 255 bytes for the rpath, but the path will be expanded by realpath() which can return a sring up to a maximum of PATH_MAX bytes. And, so as a result, if long path names are used or while creating playlists with multiple files we get a coredump with the error:

*** buffer overflow detected ***: terminated
Aborted (core dumped)

Lets use PATH_MAX for rpath length so that we have buffer for the maximum return from realpath().

Reported in Ubuntu at https://bugs.launchpad.net/ubuntu/+source/xmms2/+bug/2018449

[trofi]

Looks great! Merged.

Thank you!

We'll need to add a runtime check for overflow as well.

[waveform80]

One thing slightly concerns me with this patch is that it's fixing one instance of XMMS_PATH_MAX, but there's several uses of it scattered throughout the code that look like they could potentially be broken by longer paths. Would it not be preferable to #define XMMS_PATH_MAX PATH_MAX in xmmsc_util.h to redefine it everywhere?

I must admit I haven't tried this change, and it may break other things; I just came here from the linked Ubuntu bug while reviewing the update there, but it seems to me this might be a more wide-ranging issue than the one file patched here.

[trofi]

Yeah, generally hardcoding arbitrary limits like XMMS_PATH_MAX is problematic. Especially in the structs passing around. Ideally those all should be dynamically allocated to hold enough storage.

I'm not sure how portable PATH_MAX is. Does it have a reliable value on windows? I had an impression windows uses MAX_PATH (which is also not quite the max length for UNC paths).

[Malvineous]

It's been a long time since I've done Windows development but I remember back on Windows 95, MAX_PATH was in use. Seems that's still the case albeit there are workarounds for longer paths now if you call Unicode-specific functions.