DHT: Reply a non 20-bytes long tokens
ClosedPublic

Authored by trufanov on Jun 3 2020, 8:34 AM.

Details

Summary

I think I've noticed a bug with DHT. According to my estimations a half of DHT tokens my KTorrent client received have length < 20 (in fact mostly are 8 or 4 bytes). BEP 5 is unclear on token size. In their examples of DHT get_peers and announce_peer messages they use "token":"aoeusnth" which is only 8 bytes long.

It seems libtorrent is using just a string to store token's value. I also found some DHT crawler code on github that uses 8 byte tokens made from ipv4.

In case of libKTorrent the bt::Key is used to store the token (getpeersrsp.cpp:117) as well as info_hash. Which seems to be wrong. bt::Key constructor receives data less when 20 bytes long and doesn't initialize the rest of internal hash. And it doesn't remember the real size of the key it stored. So when libktorrent writes the response - it writes 20 bytes: the token + 00's in the end (00's are set by SHA1Hash::SHA1Hash() base constructor). The size of write is hardcoded to 20 (announcereq.cpp:68, getpeersrsp.cpp:86). As a result - recipient most probably ignores this reply as token's length it got back doesn't not match to token's length it had sent before.

I suggest to refuse from storing tokens as a fixed length bt::Key as they don't need the operator +/- or distance() calculation. And start using just QByteArray of variable size for them. My patch also limits the max size of the token that libktorrent reads to 40 bytes - just in case of malformed DHT message.

Diff Detail

Repository
R472 KTorrent Library
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
trufanov requested review of this revision.Jun 3 2020, 8:34 AM
trufanov created this revision.
stikonas accepted this revision.Jun 3 2020, 11:06 AM

Looks fine. Although, personally I would rather require Qt 5.9 instead of copying implementation. Qt 5.9 is not exactly new.

This revision is now accepted and ready to land.Jun 3 2020, 11:06 AM
This revision was automatically updated to reflect the committed changes.