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

Buffer overflow bug in MIKMIDISystemExclusiveCommand.manufacturerID #197

Closed
snej opened this issue May 30, 2017 · 1 comment
Closed

Buffer overflow bug in MIKMIDISystemExclusiveCommand.manufacturerID #197

snej opened this issue May 30, 2017 · 1 comment
Assignees
Labels
Milestone

Comments

@snej
Copy link

snej commented May 30, 2017

The implementation of MIKMIDISystemExclusiveCommand.manufacturerID has a couple of problems:

    NSUInteger manufacturerIDLocation = _has3ByteManufacturerID ? 2 : 1;
    NSUInteger manufacturerIDLength = _has3ByteManufacturerID ? 2 : 1;
    NSData *idData = [self.internalData subdataWithRange:NSMakeRange(manufacturerIDLocation, manufacturerIDLength)];
    return *(UInt32 *)[idData bytes];

First is that it creates a 1- or 2-byte NSData object and then reads 4 bytes from its buffer, which is guaranteed to overrun the buffer. If you're lucky the bytes past the end will be zeroes and the result will be correct. If not, the result will be garbage. Worst case, you fall off the end of mapped memory and crash. (Also, while I haven't tried it, this is probably guaranteed to trigger the Address Sanitizer.)

The second problem is that it assumes the number is stored in the CPU's native byte order. I don't know the MIDI spec, but most cross-platform data formats/protocols use big-endian encoding, while all current Macs and iOS devices use little-endian.

Since all that needs to be read is at most a 2-byte number, I suggest just reading the individual bytes (as uint8_t) and combining them with a shift and OR.

@armadsen armadsen added the bug label Aug 21, 2017
@armadsen armadsen self-assigned this Aug 21, 2017
@armadsen armadsen added this to the 1.6.2 milestone Aug 21, 2017
@armadsen
Copy link
Member

Fixed by 784a9c4. Thanks!

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

No branches or pull requests

2 participants