xorl %eax, %eax

CVE-2010-0012: Transmission BitTorrent Directory Traversal

leave a comment »

On December 2009, Dan Rosenberg reported this vulnerability in detail to bugs.launchpad.net. Transmission 1.77 was patched to fix this issue according to their ChangeLog entry which is available from the web here. Here is the buggy code from libtransmission/metainfo.c as seen in 1.76 release of the open source BitTorrent client…

static tr_bool
getfile( char        ** setme,
         const char   * root,
         tr_benc      * path )
{
    tr_bool success = FALSE;

    if( tr_bencIsList( path ) )
    {
        struct evbuffer * buf = evbuffer_new( );
        int               n = tr_bencListSize( path );
        int               i;

        evbuffer_add( buf, root, strlen( root ) );
        for( i = 0; i < n; ++i )
        {
            const char * str;
            if( tr_bencGetStr( tr_bencListChild( path, i ), &str )
              && strcmp( str, ".." ) )
            {
                evbuffer_add( buf, TR_PATH_DELIMITER_STR, 1 );
                evbuffer_add( buf, str, strlen( str ) );
            }
        }

        *setme = tr_utf8clean( (char*)EVBUFFER_DATA( buf ), EVBUFFER_LENGTH( buf ), NULL );
        /* fprintf( stderr, "[%s]\n", *setme ); */
        evbuffer_free( buf );
        success = TRUE;
    }

    return success;
}

This is the function that retrieves the filenames from a torrent file. Assuming that the path is list encoded, the code will fall in the ‘if’ clause and after obtaining a new pointer for buffer ‘buf’ and initializing the ‘n’ with the list’s size, it will call evbuffer_add() to add ‘root’ string to the newly allocated buffer.
After doing so, it will enter a ‘for’ loop that will retrieve the strings of the list and if those don’t match with “..”, it will add ‘TR_PATH_DELIMITER_STR’ to the buffer and then the string which is stored in ‘str’. The ‘TR_PATH_DELIMITER_STR’ is defined in libtransmission/platform.h like this:

#if defined( WIN32 )
 #define TR_PATH_DELIMITER '\\'
 #define TR_PATH_DELIMITER_STR "\\"
#else
 #define TR_PATH_DELIMITER '/'
 #define TR_PATH_DELIMITER_STR "/"
#endif

So, this is the only check against relative paths that could result in overwriting arbitrary files in the system. To fix this design flaw, a new routine was added like this:

 static tr_bool
-getfile( char        ** setme,
-         const char   * root,
-         tr_benc      * path )
+path_is_suspicious( const char * path )
+{
+    return ( path == NULL )
+        || ( strstr( path, "../" ) != NULL );
+}
+
+static tr_bool
+getfile( char ** setme, const char * root, tr_benc * path )
 {

This function will simply check that the ‘path’ passed to it doesn’t contain “../” strings and that’s it’s not NULL. Now, getfile() was changed since the ‘n’ variable that contains the size of the list is now a constant value like this:

     if( tr_bencIsList( path ) )
     {
+        int i;
+        const int n = tr_bencListSize( path );
         struct evbuffer * buf = evbuffer_new( );
-        int               n = tr_bencListSize( path );
-        int               i;
 
         evbuffer_add( buf, root, strlen( root ) );

And inside the ‘for’ loop the strcmp(3) call was completely removed…

             const char * str;
-            if( tr_bencGetStr( tr_bencListChild( path, i ), &str )
-              && strcmp( str, ".." ) )
+            if( tr_bencGetStr( tr_bencListChild( path, i ), &str ) )
             {

And pointer ‘setme’ that contains the final string as you can read in getfile() is the one that’s being checked against the directory traversal string like this:

+    if( ( *setme != NULL ) && path_is_suspicious( *setme ) )
+    {
+        tr_free( *setme );
+        *setme = NULL;
+        success = FALSE;
+    }
+
     return success;
 }

At last, inside parseFiles() a similar check takes place when single file mode is used like this:

     else if( tr_bencGetInt( length, &len ) ) /* single-file mode */
     {
+        if( path_is_suspicious( inf->name ) )
+            return "path";
+
         inf->isMultifile      = 0;

On his bug report, Dan Rosenberg provided a PoC to demonstrate the vulnerability. He suggests to create a simple file in ~/ like the one provided by him (named EVIL) which is available here. Then, download and open the torrent file he constructed which is also available here.
If the torrent file is opened from a child directory such as ~/Desktop/ it will traverse one directory up and overwrite whatever a file named ‘EVIL’ contains since it contains the following:

d8:announce39:http://tracker.publicbt.com:80/announce10:created by24:Transmission/1.75 (9117)13:creation datei1261348997e8:encoding5:UTF-84:infod6:lengthi19e4:name7:../EVIL12:piece lengthi32768e6:pieces20:�^k&@>��W���,ɓ7:privatei0eee

Which among others has: “name7:../EVIL12:piece”. As Dan Rosenberg pointed out, this could be used to overwrite “../.ssh/authorized_keys” or other sensitive files of the system.

Written by xorl

January 11, 2010 at 20:39

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