From 410dffd211af69894240f4d19c457af5af337760 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 30 Jan 2008 15:39:41 +0000 Subject: [PATCH] part 1 of the bencode exploit fix: - better error checking for int & string parsing - add automated unit tests --- libtransmission/Makefile.am | 3 + libtransmission/bencode-test.c | 151 +++++++++++++++++++++++++++++++++ libtransmission/bencode.c | 88 +++++++++++++++++++ libtransmission/bencode.h | 20 +++++ 4 files changed, 262 insertions(+) create mode 100644 libtransmission/bencode-test.c diff --git a/libtransmission/Makefile.am b/libtransmission/Makefile.am index d64ff21c1..cf2654a62 100644 --- a/libtransmission/Makefile.am +++ b/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 diff --git a/libtransmission/bencode-test.c b/libtransmission/bencode-test.c new file mode 100644 index 000000000..ae673acaf --- /dev/null +++ b/libtransmission/bencode-test.c @@ -0,0 +1,151 @@ +#include +#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; +} diff --git a/libtransmission/bencode.c b/libtransmission/bencode.c index 8eda722ce..9f4d474dd 100644 --- a/libtransmission/bencode.c +++ b/libtransmission/bencode.c @@ -24,6 +24,7 @@ #include #include /* isdigit, isprint */ +#include #include #include #include @@ -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: + * : + * 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 */ diff --git a/libtransmission/bencode.h b/libtransmission/bencode.h index 37826431f..acc13c584 100644 --- a/libtransmission/bencode.h +++ b/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