part 1 of the bencode exploit fix:

- better error checking for int & string parsing
 - add automated unit tests

libtransmission/Makefile.am

@@ -73,6 +73,7 @@ noinst_HEADERS = \
 
 
 noinst_PROGRAMS = \
+    bencode-test \
     test-fastset \
     test-peer-id
 
@@ -85,6 +86,8 @@ TEST_LDADD = \
     $(top_builddir)/third-party/libevent/libevent.la \
     $(OPENSSL_LIBS) $(PTHREAD_LIBS) -lm
 
+bencode_test_SOURCES = bencode-test.c
+bencode_test_LDADD = $(TEST_LDADD)
 test_fastset_SOURCES = test-fastset.c
 test_fastset_LDADD = $(TEST_LDADD)
 test_peer_id_SOURCES = test-peer-id.c

libtransmission/bencode-test.c

@@ -0,0 +1,151 @@
+#include <stdio.h>
+#include "transmission.h"
+#include "bencode.h"
+#include "utils.h" /* tr_free */
+
+#define VERBOSE 1
+
+int test = 0;
+
+#define check(A) { \
+    ++test; \
+    if (A) { \
+        if( VERBOSE ) \
+            fprintf( stderr, "PASS test #%d (%s, %d)\n", test, __FILE__, __LINE__ ); \
+    } else { \
+        if( VERBOSE ) \
+            fprintf( stderr, "FAIL test #%d (%s, %d)\n", test, __FILE__, __LINE__ ); \
+        return test; \
+    } \
+}
+
+static int
+testInt( void )
+{
+    uint8_t buf[128];
+    int64_t val;
+    int err;
+    const uint8_t * end;
+
+    /* good int string */
+    snprintf( (char*)buf, sizeof( buf ), "i64e" );
+    err = tr_bencParseInt( buf, 4, &end, &val );
+    check( err == 0 );
+    check( val == 64 );
+    check( end == buf + 4 );
+
+    /* missing 'e' */
+    end = NULL;
+    val = 888;
+    err = tr_bencParseInt( buf, 3, &end, &val );
+    check( err == TR_ERROR ); 
+    check( val == 888 );
+    check( end == NULL );
+
+    /* empty buffer */
+    err = tr_bencParseInt( buf, 0, &end, &val );
+    check( err == TR_ERROR ); 
+    check( val == 888 );
+    check( end == NULL );
+
+    /* bad number */
+    snprintf( (char*)buf, sizeof( buf ), "i6z4e" );
+    err = tr_bencParseInt( buf, 4, &end, &val );
+    check( err == TR_ERROR );
+    check( val == 888 );
+    check( end == NULL );
+
+    /* negative number */
+    snprintf( (char*)buf, sizeof( buf ), "i-3e" );
+    err = tr_bencParseInt( buf, 4, &end, &val );
+    check( err == TR_OK );
+    check( val == -3 );
+    check( end == buf + 4 );
+
+    /* zero */
+    snprintf( (char*)buf, sizeof( buf ), "i0e" );
+    err = tr_bencParseInt( buf, 4, &end, &val );
+    check( err == TR_OK );
+    check( val == 0 );
+    check( end == buf + 3 );
+
+    /* no leading zeroes allowed */
+    val = 0;
+    end = NULL;
+    snprintf( (char*)buf, sizeof( buf ), "i04e" );
+    err = tr_bencParseInt( buf, 4, &end, &val );
+    check( err == TR_ERROR );
+    check( val == 0 );
+    check( end == NULL );
+
+    return 0;
+}
+
+static int
+testStr( void )
+{
+    uint8_t buf[128];
+    int err;
+    const uint8_t * end;
+    uint8_t * str;
+    size_t len;
+
+    /* good string */
+    snprintf( (char*)buf, sizeof( buf ), "4:boat" );
+    err = tr_bencParseStr( buf, 6, &end, &str, &len );
+    check( err == TR_OK );
+    check( !strcmp( (char*)str, "boat" ) );
+    check( len == 4 );
+    check( end == buf + 6 );
+    tr_free( str );
+    str = NULL;
+    end = NULL;
+    len = 0;
+
+    /* string goes past end of buffer */
+    err = tr_bencParseStr( buf, 5, &end, &str, &len );
+    check( err == TR_ERROR );
+    check( str == NULL );
+    check( end == NULL );
+    check( !len );
+
+    /* empty string */
+    snprintf( (char*)buf, sizeof( buf ), "0:" );
+    err = tr_bencParseStr( buf, 2, &end, &str, &len );
+    check( err == TR_OK );
+    check( !*str );
+    check( !len );
+    check( end == buf + 2 );
+    tr_free( str );
+    str = NULL;
+    end = NULL;
+    len = 0;
+
+    /* short string */
+    snprintf( (char*)buf, sizeof( buf ), "3:boat" );
+    err = tr_bencParseStr( buf, 6, &end, &str, &len );
+    check( err == TR_OK );
+    check( !strcmp( (char*)str, "boa" ) );
+    check( len == 3 );
+    check( end == buf + 5 );
+    tr_free( str );
+    str = NULL;
+    end = NULL;
+    len = 0;
+
+    return 0;
+}
+ 
+int
+main( void )
+{
+    int i;
+
+    if(( i = testInt( ) ))
+        return i;
+
+    if(( i = testStr( ) ))
+        return i;
+
+    return 0;
+}

libtransmission/bencode.c

@@ -24,6 +24,7 @@
 
 #include <assert.h>
 #include <ctype.h> /* isdigit, isprint */
+#include <errno.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -57,6 +58,93 @@ tr_bencIsDict( const benc_val_t * val ) {
 ***
 **/
 
+/**
+ * The initial i and trailing e are beginning and ending delimiters.
+ * You can have negative numbers such as i-3e. You cannot prefix the
+ * number with a zero such as i04e. However, i0e is valid.  
+ * Example: i3e represents the integer "3" 
+ * NOTE: The maximum number of bit of this integer is unspecified,
+ * but to handle it as a signed 64bit integer is mandatory to handle
+ * "large files" aka .torrent for more that 4Gbyte 
+ */
+int
+tr_bencParseInt( const uint8_t  * buf,
+                 size_t           buflen,
+                 const uint8_t ** setme_end, 
+                 int64_t        * setme_val )
+{
+    int err = TR_OK;
+    char * endptr;
+    const void * begin;
+    const void * end;
+    int64_t val;
+
+    if( !buflen )
+        return TR_ERROR;
+    if( *buf != 'i' )
+        return TR_ERROR;
+
+    begin = buf + 1;
+    end = memchr( begin, 'e', buflen-1 );
+    if( end == NULL )
+        return TR_ERROR;
+
+    errno = 0;
+    val = strtoll( begin, &endptr, 10 );
+    if( errno || ( endptr != end ) ) /* incomplete parse */
+        err = TR_ERROR;
+    else if( val && *(const char*)begin=='0' ) /* the spec forbids leading zeroes */
+        err = TR_ERROR;
+    else {
+        *setme_end = end + 1;
+        *setme_val = val;
+    }
+
+    return err;
+}
+
+
+/**
+ * Byte strings are encoded as follows:
+ * <string length encoded in base ten ASCII>:<string data>
+ * Note that there is no constant beginning delimiter, and no ending delimiter.
+ * Example: 4:spam represents the string "spam" 
+ */
+int
+tr_bencParseStr( const uint8_t  * buf,
+                 size_t           buflen,
+                 const uint8_t ** setme_end,
+                 uint8_t       ** setme_str,
+                 size_t         * setme_strlen )
+{
+    size_t len;
+    const void * end;
+    char * endptr;
+
+    if( !buflen )
+        return TR_ERROR;
+
+    if( !isdigit( *buf  ) )
+        return TR_ERROR;
+
+    end = memchr( buf, ':', buflen );
+    if( end == NULL )
+        return TR_ERROR;
+
+    errno = 0;
+    len = strtoul( (const char*)buf, &endptr, 10 );
+    if( errno || endptr!=end )
+        return TR_ERROR;
+
+    if( ( (const uint8_t*)end - buf ) + 1 + len > buflen )
+        return TR_ERROR;
+
+    *setme_end = end + 1 + len;
+    *setme_str = (uint8_t*) tr_strndup( end + 1, len );
+    *setme_strlen = len;
+    return TR_OK;
+}
+
 /* setting to 1 to help expose bugs with tr_bencListAdd and tr_bencDictAdd */
 #define LIST_SIZE   20 /* number of items to increment list/dict buffer by */
 

libtransmission/bencode.h

@@ -53,6 +53,7 @@ typedef struct benc_val_s
     } val;
 } benc_val_t;
 
+
 #define tr_bencLoad(b,l,v,e) _tr_bencLoad((char*)(b),(l),(v),(char**)(e))
 int          _tr_bencLoad( char * buf, int len, benc_val_t * val,
                            char ** end );
@@ -89,4 +90,23 @@ char*  tr_bencSave( const benc_val_t * val, int * len );
 
 int64_t  tr_bencGetInt ( const benc_val_t * val );
 
+
+/**
+***  Treat these as private -- they're only made public here
+***  so that the unit tests can find them
+**/
+
+int  tr_bencParseInt( const uint8_t  * buf,
+                      size_t           buflen,
+                      const uint8_t ** setme_end, 
+                      int64_t        * setme_val );
+
+int  tr_bencParseStr( const uint8_t  * buf,
+                      size_t           buflen,
+                      const uint8_t ** setme_end, 
+                      uint8_t       ** setme_str,
+                      size_t         * setme_strlen );
+
+
+
 #endif