CODEWITHSUNDEEP
cbittorrent
rps
rps

Reputation: 1303

Where's the bug in this BitTorrent client's piece verification process?

I've been working on writing a BitTorrent client in C in my spare time. However -- seemingly at random -- my client will fail to verify that a received piece of a torrent is correct with the message: 

Failed to verify piece: #4

Background: a .torrent file contains the relevant metadata for the file-to-be-downloaded, including a SHA1 hash of each piece (a block of the file-to-be-downloaded), so -- after downloading a piece -- I take its SHA1 hash and compare it to the one provided by the torrent's metadata file.

Here's the code that handles verification:

 int verify_piece (void* addr, uint64_t piece_length, unsigned char* piece_sha)
 {
     SHA_CTX sha;
     unsigned char sha1result[20];

     SHA1_Init(&sha);
     SHA1_Update(&sha, addr, piece_length);
     SHA1_Final(sha1result, &sha);

     if (memcmp(sha1result, piece_sha, 20)) {
          printf("Expected: ");
          print_sha1(piece_sha);
          printf("Received: ");
          print_sha1(sha1result);
          return 0;
     } else
          return 1;
  }

In an attempt to track down the bug, I rigged up two functions: write_incorrect_piece() and write_correct_piece(). The first function, write_incorrect_piece() is called when verify_piece() fails. It writes the piece that failed verification to a file so that I can inspect it with hexdump.

 void write_incorrect_piece (void* piece, uint32_t piece_length, uint32_t index)
 {
     FILE* failed_piece = fopen("failed_piece.out", "w+");

     write(fileno(failed_piece), piece, piece_length);
     fclose(failed_piece);
     printf("ERROR: Wrote piece #%d to failed_piece.out for inspection with hexdump.\n",
            index);

     write_correct_piece(piece_length, index);
     exit(1);
 }

As you can see, write_incorrect_piece() takes the parameters void* piece, which is a pointer to the piece which failed verification, uint32_t piece_length, which is the length of the piece, and uint32_t index which is the index of the piece which failed verification. It then copies the incorrect piece to a file called failed_piece.out.

You'll notice that before calling exit(), write_incorrect_piece() calls the function write_correct_piece(). This is the second function I wrote to help with debugging this error. It takes a known good version of the file that the torrent client is attempting to download (known_good.iso) and then copies the relevant piece to a file (correct_piece.out) so that I can compare it to the file containing the incorrect piece.

void write_correct_piece (uint32_t piece_length, uint32_t index)
{
     FILE* known_good = fopen("known_good.iso", "r+");
     FILE* correct_piece = fopen("correct_piece.out", "w+");

     /* get file size for MMAPing */
     struct stat st;
     stat("known_good.iso", &st);
     long size = st.st_size;

     void* addr = mmap(0,
                       size,
                       PROT_READ | PROT_WRITE,
                       MAP_SHARED,
                       fileno(known_good),
                       0);

     write(fileno(correct_piece), addr + piece_length * index, piece_length);
     munmap(addr, size);
     fclose(known_good);
     fclose(correct_piece);
}

I then took the two written files and formatted them with hexdump so that the output would be human readable and then tried to compare them via diff, like so

$ hexdump failed_piece.out > failed_piece.dump
$ hexdump correct_piece.out > correct_piece.dump
$ diff correct_piece.dump failed_piece.dump

To my surprise, diff didn't output anything. The files are exactly the same. Why, then, was the SHA1 hash of the piece different from the one expected by the torrent file? Perhaps, I then thought, the SHA1 hash from the metadata file was incorrect. If that's the case, I would expect the client to always fail to verify piece #4, so I modified the torrent client's scheduler to only attempt to download piece #4 to see if it does, in fact, always fail verification. 

Successfully downloaded piece: #4

Wtf? Now I'm at an impasse. I figure a couple of different things could be causing this:

For the life of me, however, I can't find any problems with any of the above code, so I'm hoping you fine folks can point out what I'm missing.

The entire source of the client is at https://github.com/robertseaton/slug.

Edit: Replaced strncmp() with memcmp() in verify_piece() and cleaned up both write_incorrect_piece() and write_correct_piece().

Edit: Here's an example of the SHA1 hashes on a failed piece that seems correct when I manually diff it. As you can see, the hashes are not similar at all:

Expected: 239 66 216 164 16 120 73 24 1 236 116 173 144 85 243 152 160 165 64 231 
Received: 214 94 49 185 54 159 255 201 214 137 102 23 223 76 102 138 89 94 154 69 
Failed to verify piece: #13

Upvotes: 1

Views: 350

Answers (1)

rps
rps

Reputation: 1303

Okay, so if we have a look at the invocation of verify_piece(), we see this:

 if (verify_piece(p->torrent->mmap + index * p->torrent->piece_length,
     p->torrent->piece_length, p->torrent->pieces[index].sha1)) {

Now, trivially, we know that the first and second parameters are correct since it is later reused when the program calls write_incorrect_piece() and we have already verified that its output is correct. So, we are free to focus on the third parameter, p->torrent->pieces[index].sha1.

At first glance, the parameter looks correct, since we are using index throughout the function. However, consider, what if index -- while perfectly valid on a correctly sorted array -- is being used as an index to an array that no longer contains the pieces in the assumed order?

Viola! Here's the culprit (in __schedule()):

qsort(t->pieces, t->num_pieces, sizeof(struct Piece), rarest);

Comment out the call to qsort() and the client runs as expected.

Upvotes: 1

Related Questions