Too prone to buffer overflows. Yesterday, following the trails of a bug reported on #xine @ Freenode, I ended up looking at xine-ui’s code for handling strings. Not like xine-ui’s first task is to handle strings, but it does a lot of string parsing for handling commands sent via network, to handle LIRC commands and to handle the errors.
Well, what the problem was? A part the quantity of format string vulnerabilities in the latest released version (0.99.4), that I fixed before security advisories were released for that (just an year before), there were some really nasty codepaths that used functions without boundaries checking, which were like a written invite to a buffer overflow (although none of them worked with user-supplied input, as far as I could see, so they are not vulnerabilities).
I ended up adding an strlcat() function from OpenBSD and using that to replace a good deal of strcat() functions, and then I tried to simplify usage of printf(), puts and so on. If you ever tried to refactor the string handling core of a C program that has lot of similar code, you know that this is a way to ask for trouble, considering it’s easy to confuse one situation for another, luckily, Darren helped me by checking all the patches, and improving them a bit.
The new, more solid, although probably not-yet-enough, version of xine-ui is in portage as a new snapshot, I also added it because the last snapshot didn’t build on FreeBSD. Hopefully if there is any security issue in that code, it will be easier for us to mark that stable. Also, with the new snapshot, maybe because of the libtool refactor but I’m not sure, there are no more symbols exported by the xine binary, so I simply dropped the check that used -fvisibility=hidden on GCC 4.1.1 to remove them.
Now, if I’ll ever get enough bored, I could write ruby-xine bindings, and then write a toolkit-agnostic xine frontend in Ruby 😛 Yeah yeah I know, lately I’ve been doing anything in Ruby, but it’s not my fault if it’s a good language…
My university thesis deals with C string handling using something resembling a library to manage strings, inspired by vsftpd’s mystr string functions and Postfix’s vstring and vbuf design.Maybe xine, like many others, should switch to using a “secure” string wrapper all over the place. It provides for added security in the long run.