Changeset 4892

Timestamp:

Feb 1, 2008, 3:50:12 PM (14 years ago)

Author:

charles

Message:

(1.0x) fix the 1.04 bencode errors reported in the forums this morning. add regression tests.

Location:

branches/1.0x/libtransmission

Files:

• bencode-test.c (modified) (10 diffs)
• bencode.c (modified) (16 diffs)

branches/1.0x/libtransmission/bencode-test.c

@@ -25 +25 @@
     uint8_t buf[128];
     int64_t val;
-    unsigned int err;
+    int err;
     const uint8_t * end;
 
@@ -39 +39 @@
     val = 888;
     err = tr_bencParseInt( buf, buf+3, &end, &val );
-    check( err == TR_ERROR ); 
+    check( err == (int)TR_ERROR ); 
     check( val == 888 );
     check( end == NULL );
@@ -45 +45 @@
     /* empty buffer */
     err = tr_bencParseInt( buf, buf+0, &end, &val );
-    check( err == TR_ERROR ); 
+    check( err == (int)TR_ERROR ); 
     check( val == 888 );
     check( end == NULL );
@@ -52 +52 @@
     snprintf( (char*)buf, sizeof( buf ), "i6z4e" );
     err = tr_bencParseInt( buf, buf+5, &end, &val );
-    check( err == TR_ERROR );
+    check( err == (int)TR_ERROR );
     check( val == 888 );
     check( end == NULL );
@@ -75 +75 @@
     snprintf( (char*)buf, sizeof( buf ), "i04e" );
     err = tr_bencParseInt( buf, buf+4, &end, &val );
-    check( err == TR_ERROR );
+    check( err == (int)TR_ERROR );
     check( val == 0 );
     check( end == NULL );
@@ -86 +86 @@
 {
     uint8_t buf[128];
-    unsigned int err;
+    int err;
     const uint8_t * end;
     uint8_t * str;
@@ -105 +105 @@
     /* string goes past end of buffer */
     err = tr_bencParseStr( buf, buf+5, &end, &str, &len );
-    check( err == TR_ERROR );
+    check( err == (int)TR_ERROR );
     check( str == NULL );
     check( end == NULL );
@@ -138 +138 @@
 
 static int
+testString( const char * str, int isGood )
+{
+    benc_val_t val;
+    const uint8_t * end = NULL;
+    char * saved;
+    const size_t len = strlen( str );
+    int savedLen;
+    int err = tr_bencParse( str, str+len, &val , &end );
+    if( !isGood ) {
+        check( err );
+    } else {
+        check( !err );
+        check( end == (const uint8_t*)str + len );
+        saved = tr_bencSave( &val, &savedLen );
+        check( !strcmp( saved, str ) );
+        check( len == (size_t)savedLen );
+        tr_free( saved );
+        tr_bencFree( &val );
+    }
+    return 0;
+}
+
+static int
 testParse( void )
 {
@@ -185 +208 @@
     tr_bencFree( &val );
 
-    end = NULL;
-    snprintf( (char*)buf, sizeof( buf ), "llleee" );
-    err = tr_bencParse( buf, buf + sizeof( buf ), &val , &end );
-    check( !err );
-    check( end == buf + 6 );
-    saved = tr_bencSave( &val, &len );
-    check( !strcmp( saved, "llleee" ) );
-    tr_free( saved );
-    tr_bencFree( &val );
+    if(( err = testString( "llleee", TRUE )))
+        return err;
+    if(( err = testString( "d3:cow3:moo4:spam4:eggse", TRUE )))
+        return err;
+    if(( err = testString( "d4:spaml1:a1:bee", TRUE )))
+        return err;
+#if 0
+    if(( err = testString( "d9:publisher3:bob18:publisher.location4:home17:publisher-webpage15:www.example.come", TRUE )))
+        return err;
+#endif
+    if(( err = testString( "d8:completei1e8:intervali1800e12:min intervali1800e5:peers0:e", TRUE )))
+        return err;
+    if(( err = testString( "d1:ai0e1:be", FALSE ))) /* odd number of children */
+        return err;
+    if(( err = testString( "", FALSE )))
+        return err;
 
     /* nested containers
@@ -208 +238 @@
     check( !strcmp( saved, "lld1:ai64e1:bi32eeee" ) );
     tr_free( saved );
-    tr_bencFree( &val );
-
-    end = NULL;
-    snprintf( (char*)buf, sizeof( buf ), "d8:completei1e8:intervali1800e12:min intervali1800e5:peers0:e" );
-    err = tr_bencParse( buf, buf+sizeof( buf ), &val, &end );
-    check( !err );
-    check( end == buf + strlen( (const char*)buf ) );
     tr_bencFree( &val );
 

branches/1.0x/libtransmission/bencode.c

@@ -30 +30 @@
 #include <stdlib.h>
 
-#include <event.h>
+#include <event.h> /* evbuffer */
 
 #include "transmission.h"
 #include "bencode.h"
 #include "ptrarray.h"
-#include "utils.h"
+#include "utils.h" /* tr_new(), tr_free() */
 
 /**
@@ -42 +42 @@
 
 static int
-tr_bencIsInt( const benc_val_t * val )
-{
-    return val!=NULL && val->type==TYPE_INT;
-}
-
-static int
-tr_bencIsList( const benc_val_t * val )
-{
-    return val!=NULL && val->type==TYPE_LIST;
-}
-
-static int
-tr_bencIsDict( const benc_val_t * val )
-{
-    return val!=NULL && val->type==TYPE_DICT;
-}
+isType( const benc_val_t * val, int type )
+{
+    return ( ( val != NULL ) && ( val->type == type ) );
+}
+
+#define isInt(v)    ( isType( ( v ), TYPE_INT ) )
+#define isString(v) ( isType( ( v ), TYPE_STR ) )
+#define isList(v)   ( isType( ( v ), TYPE_LIST ) )
+#define isDict(v)   ( isType( ( v ), TYPE_DICT ) )
 
 static int
 isContainer( const benc_val_t * val )
 {
-    return val!=NULL && ( val->type & ( TYPE_DICT | TYPE_LIST ) );
+    return isList(val) || isDict(val);
+}
+static int
+isSomething( const benc_val_t * val )
+{
+    return isContainer(val) || isInt(val) || isString(val);
 }
 
@@ -69 +67 @@
 {
     benc_val_t * ret = NULL;
-    if( tr_bencIsList( val ) && ( i >= 0 ) && ( i < val->val.l.count ) )
+    if( isList( val ) && ( i >= 0 ) && ( i < val->val.l.count ) )
         ret = val->val.l.vals + i;
     return ret;
@@ -215 +213 @@
 
 /**
- * this function's awkward stack-based implementation
- * is to prevent maliciously-crafed bencode data from
- * smashing our stack through deep recursion. (#667)
+ * This function's previous recursive implementation was
+ * easier to read, but was vulnerable to a smash-stacking
+ * attack via maliciously-crafted bencoded data. (#667)
  */
 int
@@ -230 +228 @@
     tr_ptrArray * parentStack = tr_ptrArrayNew( );
 
+    tr_bencInit( top, 0 );
+
     while( buf != bufend )
     {
@@ -275 +275 @@
         else if( *buf=='e' ) /* end of list or dict */
         {
+            benc_val_t * node;
             ++buf;
             if( tr_ptrArrayEmpty( parentStack ) )
                 return TR_ERROR;
+
+            node = tr_ptrArrayBack( parentStack );
+            if( isDict( node ) && ( node->val.l.count % 2 ) )
+                return TR_ERROR; /* odd # of children in dict */
+
             tr_ptrArrayPop( parentStack );
             if( tr_ptrArrayEmpty( parentStack ) )
@@ -309 +315 @@
     }
 
-    err = tr_ptrArrayEmpty( parentStack ) ? 0 : 1;
+    err = !isSomething( top ) || !tr_ptrArrayEmpty( parentStack );
 
     if( !err && ( setme_end != NULL ) )
@@ -368 +374 @@
 tr_bencGetInt ( const benc_val_t * val )
 {
-    assert( tr_bencIsInt( val ) );
+    assert( isInt( val ) );
     return val->val.i;
 }
@@ -454 +460 @@
     benc_val_t * item;
 
-    assert( tr_bencIsList( list ) );
+    assert( isList( list ) );
     assert( list->val.l.count < list->val.l.alloc );
 
@@ -469 +475 @@
     benc_val_t * keyval, * itemval;
 
-    assert( tr_bencIsDict( dict ) );
+    assert( isDict( dict ) );
     assert( dict->val.l.count + 2 <= dict->val.l.alloc );
 
@@ -503 +509 @@
 {
     const benc_val_t * val;
-    int valIsSaved;
+    int valIsVisited;
     int childCount;
     int childIndex;
@@ -512 +518 @@
 nodeNewDict( const benc_val_t * val )
 {
-    int i, j, n;
+    int i, j;
+    int nKeys;
     struct SaveNode * node;
     struct KeyIndex * indices;
 
-    assert( tr_bencIsDict( val ) );
-
-    n = val->val.l.count;
+    assert( isDict( val ) );
+
+    nKeys = val->val.l.count / 2;
     node = tr_new0( struct SaveNode, 1 );
     node->val = val;
-    node->children = tr_new0( int, n );
+    node->children = tr_new0( int, nKeys * 2 );
 
     /* ugh, a dictionary's children have to be sorted by key... */
-    indices = tr_new( struct KeyIndex, n );
-    for( i=j=0; i<n; i+=2, ++j ) {
+    indices = tr_new( struct KeyIndex, nKeys );
+    for( i=j=0; i<(nKeys*2); i+=2, ++j ) {
         indices[j].key = val->val.l.vals[i].val.s.s;
         indices[j].index = i;
@@ -536 +543 @@
     }
 
-    assert( node->childCount == n );
+    assert( node->childCount == nKeys * 2 );
     tr_free( indices );
     return node;
@@ -547 +554 @@
     struct SaveNode * node;
 
-    assert( tr_bencIsList( val ) );
+    assert( isList( val ) );
 
     n = val->val.l.count;
@@ -599 +606 @@
 
 /**
- * this function's awkward stack-based implementation
- * is to prevent maliciously-crafed bencode data from
- * smashing our stack through deep recursion. (#667)
+ * This function's previous recursive implementation was
+ * easier to read, but was vulnerable to a smash-stacking
+ * attack via maliciously-crafted bencoded data. (#667)
  */
 static void
@@ -616 +623 @@
         const benc_val_t * val;
 
-        if( !node->valIsSaved )
+        if( !node->valIsVisited )
         {
             val = node->val;
-            node->valIsSaved = TRUE;
+            node->valIsVisited = TRUE;
         }
         else if( node->childIndex < node->childCount )