https://bugs.kde.org/show_bug.cgi?id=523426
Bug ID: 523426
Summary: latest libktorrent array out of bound conditions
Classification: Applications
Product: ktorrent
Version First unspecified
Reported In:
Platform: EndeavourOS
OS: Linux
Status: REPORTED
Severity: crash
Priority: NOR
Component: general
Assignee: [email protected]
Reporter: [email protected]
Target Milestone: ---
Created attachment 194550
--> https://bugs.kde.org/attachment.cgi?id=194550&action=edit
backtrace
DESCRIPTION
libktorrent commit 2f94b4cdcd8bc30218a846354887155be11c86d0 and later causes
array out of bound conditions
STEPS TO REPRODUCE
1. Build KTorrent (master) with libktorrent (master), syndication (master) and
kcodecs (master) and run with system KIO (or any other workable KIO would do
too) in debug mode
2. Load some actual torrents
3. Give it time
OBSERVED RESULT
One of the places of crash:
udptracker.cpp -> void UDPTracker::announceReceived(Int32 tid, const bt::Uint8
*buf, bt::Uint32 size)
ADDITIONAL INFORMATION
The older implementation was:
```C++
const Uint32 nip = leechers + seeders;
Uint32 j = 0;
for (Uint32 i = 20; i < size && j < nip; i += 6, j++) {
const Uint32 ip = ReadUint32(buf, i);
addPeer(net::Address(ip, ReadUint16(buf, i + 4)), false);
}
```
The newer one:
```C++
const Uint32 nip = leechers + seeders;
const auto ip_list = QByteArrayView{buf, size}.sliced(20);
for (Uint32 i = 0; i < nip; ++i) {
addPeer(net::Address::fromCompactIPv4(ip_list.sliced(i * 6, 6)),
false);
}
```
I had 2 conditions of crash.
- One, in which size of buf was only 20 and there were values for interval,
leechers and seeders, causing the for loop to crash at i == 0.
- Second one in which size of buf was 80, but nip calculated to 16, causing
crash at i == 10.
Not sure if these are malicious packets from peers, valid packets with
unhandled conditions or just libktorrent starting to parse a buffer without
receiving all of it.
EXPECTED RESULT
1. Only go forward when the whole input is available (checking the code in
readyToRead, I suppose this protocol doesn't tell the message size? That's a
bummer. If you want to do it properly, I think you can't consider using
std::move here `d->dhandler->dataReceived(std::move(buf), addr);` just yet. You
will need to keep the buffer until it has enough bytes to finish the whole
thing.)
2. I suggest an additional check, making sure that the size of buf is enough
for the protocol, before starting the loop.
BACKTRACE
Attachment
--
You are receiving this mail because:
You are watching all bug changes.
