transmission / transmission

Official Transmission BitTorrent client repository
https://transmissionbt.com
Other
11.67k stars 1.18k forks source link

peers2 and peer2-6 in resume files, incompat with earlier Transmission #6882

Open reardonia opened 3 weeks ago

reardonia commented 3 weeks ago

What is the issue?

Filing as an issue as I don't have a simple answer or fix.

During the big rewrite, the "tr_socket_address" object was introduced such that the tr_pex object has a different in-memory and on-disk size than in earlier versions (Tr3, but also earlier Tr4 versions prior to ~ f83a60830ae). This causes the packing of the tr_pex object to change from earlier revisions, expanding from 24 bytes to 28 bytes.

The code that reads & writes this binary data ( savePeers(), loadPeers() in resume.cc ) doesn't do any kind of check, it just assumes the data is correct.

I would like to revert to the earlier 24-byte packed binary field to ensure highest compat with past Transmission. If we are stuck with the new 28-byte field, then we need safety code in loadPeers()

Note also that there is a very slight leak of sensitive info in the current implementation, as we write raw uninitialized fields out of disk. There is a 3-byte pad to the 'flags' field of tr_pex that is filled to garbage.

Which application of Transmission?

transmission-daemon

Which version of Transmission?

Tr4

Pentaphon commented 3 weeks ago

Note also that there is a very slight leak of sensitive info in the current implementation, as we write raw uninitialized fields out of disk. There is a 3-byte pad to the 'flags' field of tr_pex that is filled to garbage.

Pinging @ckerr about this since it seems like a priority issue.

tearfur commented 3 weeks ago

Rather than fiddling with the struct's padding, I think tr_pex should have a method to serialise itself into a benc object. ~I'll come up with more details later.~

If we are going to care about the garbage data potentially leaking sensitive information, then we should have never relied on the compiler's alignment, because there is no standard to it.

Edit: Well, there aren't much details to be outlined after all. Distinguishing between a benc object and a raw byte string will be trivial, so it'll be very easy to identify the old peer data.