Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PacketBuffer accepts bigger packets than it should #57

Open
mzero opened this issue Feb 4, 2025 · 3 comments
Open

PacketBuffer accepts bigger packets than it should #57

mzero opened this issue Feb 4, 2025 · 3 comments

Comments

@mzero
Copy link

mzero commented Feb 4, 2025

While it is hard to discern, there is a limit in CoreMIDI on the size of a packet in a MIDIPacketList. The docs only say:

The maximum size of a packet list is 65536 bytes. Large sysex messages must be sent in smaller packet lists.

Note also that the length field of MIDIPacket is only a UInt16. The actual packet size limit is smaller than that, after subtracting out the MIDIPacket and MIDIPacketList overheads from the max size of the list.


PacketBuffer will attempt to create a MIDIPacketList with a single MIDIPacket, large enough to accommodate however big a data object is passed in. If used with very large messages (think 80k sysex!) - PacketBuffer will build the structures, but the resulting thing will be corrupted, as it can't really hold a message that big.

PacketBuffer should at least make it clear in the docs that there is a limit on size.


How did I find this? I'm sending 80k sysexs (to control a synth) from a WebMIDI app.... and in Firefox on Mac it crashes the browser completely and instantly! I don't know who to blame for the crash... but the 80k byte array gets passed:

JavaScript -> Firefox/Gekko's C++ code -> Firefox/Gekkot's Rust crate midir_impl -> Boodlnagg's Rust midir package -> coremidi

@mzero
Copy link
Author

mzero commented Feb 4, 2025

Bug filed with midir: Boddlnagg/midir#165
Bug filed with FireFox: https://bugzilla.mozilla.org/show_bug.cgi?id=1945967

@mzero
Copy link
Author

mzero commented Feb 5, 2025

Ah, further investigation implicates this code (which appears twice in PacketBuffer):

        let current_packet_ptr = unsafe {
            MIDIPacketListAdd(
                packet_list_ptr,
                storage.capacity() as u64,
                current_packet_ptr,
                timestamp,
                data.len() as u64,
                data.as_ptr(),
            )
        };
        let current_packet_offset = unsafe {
            (current_packet_ptr as *const u8).offset_from(packet_list_ptr as *const u8) as usize
        };

If the data is too big to fit, then MIDIPacketListAdd returns null (as per CoreMIDI header files)... this will cause the computation of current_packet_offset to be some giant value.

@Boddlnagg
Copy link
Contributor

The same problem exists with the call to MIDIEventListAdd in EventBuffer::push ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants