Bug: readFrameFlags applies ID3v2.4 flag semantics to ID3v2.3 frames - wrong mp3 tag decoding

[MinnieTheMoocher]

Is there an existing issue for this?

• I have searched the existing issues

music-metadata version

11.12.3

Current Behavior

readFrameFlags applies ID3v2.4 flag bit positions to ID3v2.3 frames,
but the two standards use different bit layouts for both frame status and frame format flags:

ID3v2.3 (%abc00000 %ijk00000, from id3.org/id3v2.3.0):

• Status byte: bit 7 = tag_alter_preservation, bit 6 = file_alter_preservation, bit 5 = read_only
• Format byte: bit 7 = compression, bit 6 = encryption, bit 5 = grouping_identity

ID3v2.4 (%0abc0000 %0h00kmnp, from id3.org/id3v2.4.0-structure):

• Status byte: bit 6 = tag_alter_preservation, bit 5 = file_alter_preservation, bit 4 = read_only
• Format byte: bit 6 = grouping_identity, bit 3 = compression, bit 2 = encryption, bit 1 = unsynchronisation, bit 0 = data_length_indicator

The current code unconditionally parses all versions with the v2.4 layout. For v2.3 frames, bits 0-4 of the format byte are undefined/reserved and should be ignored, but they get misinterpreted as unsynchronisation (bit 1) and data_length_indicator (bit 0). When a v2.3 frame happens to have those low bits set (e.g. any encoder that leaves reserved bits nonzero), the parser:

1. Strips 4 bytes from the frame data (false data_length_indicator)
2. Runs unsynchronisation removal (false unsynchronisation)

Both corrupt the frame payload: in the APIC case, this shifts the MIME type offset by 4 bytes, producing e.g. "e/jpeg" instead of "image/jpeg" and truncating image data.

Suggested fix

Pass majorVer into readFrameFlags and use the appropriate bit mapping for each version:

function readFrameFlags(b, majorVer) {
    if (majorVer === 3) {
        return {
            status: {
                tag_alter_preservation: util.getBit(b, 0, 7),
                file_alter_preservation: util.getBit(b, 0, 6),
                read_only: util.getBit(b, 0, 5)
            },
            format: {
                compression: util.getBit(b, 1, 7),
                encryption: util.getBit(b, 1, 6),
                grouping_identity: util.getBit(b, 1, 5),
                unsynchronisation: false,
                data_length_indicator: false
            }
        };
    }
    return {
        status: {
            tag_alter_preservation: util.getBit(b, 0, 6),
            file_alter_preservation: util.getBit(b, 0, 5),
            read_only: util.getBit(b, 0, 4)
        },
        format: {
            grouping_identity: util.getBit(b, 1, 7),
            compression: util.getBit(b, 1, 3),
            encryption: util.getBit(b, 1, 2),
            unsynchronisation: util.getBit(b, 1, 1),
            data_length_indicator: util.getBit(b, 1, 0)
        }
    };
}

And update the call site in parseFrameHeaderV23V24 line 64 (FrameHeader.js):

flags: readFrameFlags(uint8Array.subarray(8, 10), majorVer)

(Note: the v2.4 code already has a pre-existing minor issue!:
grouping_identity is read from bit 7 instead of bit 6 per spec,
but since bit 7 is reserved/unused in v2.4, this is benign.)

Example file is attached. Its original tags have been censored as "XXXX"
(but otherwise exactly kept as in the original file!),
and the original audio was replaced by silence. To reproduce:

const { parseFile } = require('music-metadata');
const { common } = await parseFile('bug-repro.mp3', { duration: false });
console.log(common.picture[0].format); // "e/jpeg" instead of "image/jpeg"

Note the broken cover art image parsing.

Expected Behavior

correct mp3 tag decoding

bug-repro.mp3

Attached audio sample?

• I have provided sufficient information to reproduce the issue

[MohammadYusif]

I've traced this to readFrameFlags (in lib/id3v2/FrameHeader.ts) using the ID3v2.4 bit layout unconditionally — for v2.3 frames the reserved low bits of the format byte get misinterpreted as data_length_indicator (bit 0) and unsynchronisation (bit 1), so ID3v2Parser.readFrameData strips 4 bytes and/or runs unsync removal and corrupts the payload (hence e/jpeg). Planning to fix by passing majorVer into readFrameFlags and branching on the version-specific bit positions (with unsync/data-length-indicator forced false for v2.3) — happy to discuss direction before opening a PR.