InputStream: missing bounds checks on malformed MTP responses

[ludovicaelbrecht]

When using Claude to do a quick check for potential issues in the MTP parsing code, it noticed a couple of robustness issues in mtp/ptp/InputStream.h that a rogue or malicious USB device could trigger.

(I'm making the assumption that potential bugs found using AI are welcome in this project.)

Issue 1: Skip() has no bounds check

Skip() advances _offset without checking whether the skip would exceed the buffer:

void Skip(size_t size)
{ _offset += size; }

If a device sends a crafted packet with a large skip value, _offset silently overflows past the buffer end. The next Read*() call using .at() will throw, but unsigned wraparound could land _offset back inside the buffer, causing subsequent reads to consume data from the wrong position.

Fix:

void Skip(size_t size)
{
    if (size > _data.size() - _offset)
        throw std::runtime_error("InputStream::Skip: out of bounds");
    _offset += size;
}

Issue 2: ReadArray() allocates based on a device-controlled size

u32 size = Read32();
while(size--) { ... array.push_back(el); }

A rogue device can send size = 0xFFFFFFFF, causing the host to attempt filling a ~4 billion element vector, leading to OOM / denial of service.

Fix:

static constexpr u32 MaxArraySize = 1024 * 1024;

u32 size = Read32();
if (size > MaxArraySize)
    throw std::runtime_error("InputStream::ReadArray: array too large");
array.reserve(size);
while(size--) { ... }

Additional, less problematic issues

• GetInterfaceStringIndex in Device.cpp reads offset + 1 before validating len, which could throw if the descriptor ends on the last byte. Also, offset += len doesn't check that offset + len stays within descData.size().
• ReadString() reads len as a u8 but silently handles len=0 inconsistently with the MTP spec (which encodes an empty string as length 1), which can desync the parser for subsequent fields.

---

All of these require a malicious or buggy USB device to trigger, no remote attack surface.