[MPlayer-dev-eng] [Suggested PATCH] Increase maximum ftp file path length

Sergey sergemp at mail.ru
Tue Nov 6 10:38:09 CET 2012


On Mon, 5 Nov 2012 09:51:19 Nicolas George wrote:

> Are there really still people using ftp in 2012?

Well, FTP is good for me because:
* It's cross-platform and easy to use. Basically, drag-n-drop in your
  favourite file manager.
* It supports both upload and download. I.e. a few privileged users
  can upload files there, and everyone else can open them.
* It's extremely efficient and fast. Even on small embedded routers
  it's limited by HDD or network speed, not by RAM or CPU.
  This is important when you watch video taken by 1920x1080 camera.

Is there anything better than FTP?

>>  static int readresp(struct stream_priv_s* ctl,char* rsp)
>>  {
>> -    static char response[256];
>> +    static char response[65536];
> Why is this array static? It is filled unconditionally each time, and
> therefore does not need to retain its contents.

Not sure. Maybe to save some stack. Or maybe it was casual. I just
left it as it was, I don't like changing things that already work. :)
Do you want me to put them all in stack?

>>      char match[5];
>>      int r;
>>  
>> -    if (readline(response,256,ctl) == -1)
>> +    if (readline(response,65536,ctl) == -1)
>>        return 0;
>
> This kind of code is fragile and a sure way of getting an exploitable buffer
> overflow at some point. If someone changes it now, they better fix it
> correctly: either use sizeof(response) instead of duplicating the size or
> use a macro for the size, or both.
> 
> Since the same change is made several times, a macro to set the size once
> and for all is probably the best idea. It would also enable to change the
> size easily if someone happens to have a problem with 64k arrays on the
> stack (I do not think anyone will with MPlayer).

Yeah, I was thinking about some global #define for that, or maybe
even dynamically allocating the required size. But then I checked log
for that file and... this code was working like that for ~10 years,
since the very first commit. Making easily changeable buffer size
looks pointless to me if nobody is going to use it for tens of years.

-- 
  Sergey


More information about the MPlayer-dev-eng mailing list