xorl %eax, %eax

CVE-2009-1440: aMule Insufficient Character Escaping

leave a comment »

This is a simple bug reported by Sam Hocevar on 21 April 2009 and affects aMule 2.2.4 release. aMule is a popular open source peer to peer file sharing software that works with various different P2P networks. Here is the buggy code as seen at src/DownloadListCtrl.cpp.

void CDownloadListCtrl::PreviewFile(CPartFile* file)
{
        wxString command;
        // If no player set in preferences, use mplayer.
        // And please, do a warning also :P
        if (thePrefs::GetVideoPlayer().IsEmpty()) {
   ...
                command = wxT("xterm -T \"aMule Preview\" -iconic -e mplayer ") QUOTE wxT("$file") QUOTE;
        } else {
                command = thePrefs::GetVideoPlayer();
        }

        // Check if we are (pre)viewing a completed file or not
        if (file->GetStatus() != PS_COMPLETE) {
   ...   
        } else {
                // This is a complete file
                // FIXME: This is probably not going to work if the filenames are mangled ...
                wxString rawFileName = file->GetFullName().GetRaw();
                if (!command.Replace(wxT("$file"), rawFileName)) {
                        // No magic string, so we just append the filename to the player command
                        // Need to use quotes in case filename contains spaces
                        command << wxT(" ") << QUOTE << rawFileName << QUOTE;
                }
        }
    ...
        // We can't use wxShell here, it blocks the app
        CTerminationProcess *p = new CTerminationProcess(command);
        int ret = wxExecute(command, wxEXEC_ASYNC, p);
    ...
}
&#91;/sourcecode&#93;

In the first if statement, if there is no video player set in the preferences, aMule will attempt to invoke mplayer in a xterm terminal simply like this:

xterm -T "aMule Preview" -iconic -e mplayer 'our_file'

Otherwise, it will use the video player defined in the preferences. If the file that is pre-viewed is complete, it will move into the second else clause. There it appends the raw file name to the command to be executed. At last, it executes the command. The vulnerability is that a user can create a file which has single quote charater ' to its filename in order to escape and execute arbitrary commands. To fix this, the following patch was applied.

&#91;sourcecode language="cpp"&#93;
                wxString rawFileName = file->GetFullName().GetRaw();
+
+#ifndef __WXMSW__
+               rawFileName.Replace(QUOTE, wxT("'\"'\"'"));
+#endif
+
                if (!command.Replace(wxT("$file"), rawFileName)) {

Using this, the single quote character is filtered from the raw filename.

Written by xorl

June 23, 2009 at 19:34

Posted in bugs

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s