mpetazzoni / ttorrent

BitTorrent Java library with tracker and download client
http://mpetazzoni.github.com/ttorrent/
Apache License 2.0
1.38k stars 502 forks source link

Any reason 'pstrlen' in handshake validation (ConnectionHandler) could return a negative number? #94

Closed c-riddell closed 10 years ago

c-riddell commented 10 years ago

I have seen this in the wild. An IllegalArgumentException is thrown when the byte buffer attempts to allocate a negative number.

My guess is that since no other pstr limit on size is specified in the BitTorrent spec (it seems), some clients have decided that the pstrlen byte is an unsigned byte (and using values above 127) instead of Javas interpretation of a 2's comp value, thereby java interpreting a negative value when above 127 (since the MSB is the sign bit in 2's comp).

So the byte may need to be normalized. Does this sound logical?

c-riddell commented 10 years ago

The following would convert from 2's comp:

    byte pstrByte = len.get();
    int pstrlen = pstrByte < 0 ? ~pstrByte + 1 : pstrByte;

However having now read the spec about pstrlength, its highly confusing as to why one would get a value high enough to use the MSB (above 127), (since "BitTorrent protocol" = length 19). Looking at the handshake parse I see an exception will be thrown if its not "BitTorrent protocol" and thus not length 19 anyway.

I suppose either some client is being a d*ck or the byte was not correctly read, perhaps across the network.

I think it is safe to close this issue.

mpetazzoni commented 10 years ago

Thanks for the investigation. Closing.