mirror of
https://github.com/taglib/taglib.git
synced 2026-08-27 12:47:01 -04:00
manuaudioandGitHub
ee0ab489e0
* ID3v2: validate the text encoding byte before casting it The first byte of most ID3v2 frames selects the text encoding and is cast straight to String::Type. It comes from the file, so it can be any value, and String::Type enumerates 0..4 — loading an enumeration object whose value is outside the enumeration's range is undefined: runtime error: load of value 127, which is not a valid value for type 'String::Type' runtime error: load of value 4294967295, which is not a valid value for type 'String::Type' The second value is 0xFF read through a plain signed char. It matters beyond the sanitizer: String::data() switches on the type with no default case, so an unrecognised encoding silently falls through and returns an empty ByteVector on the render path. Add Utils::textEncodingFromByte(), which maps the byte to its String::Type or falls back to Latin1 — the encoding these frames already declare as their default — and use it at the eleven sites that read the byte from a file. Note that a byte of 5, 6 or 7 is *not* undefined, because the range of an enumeration is the bit width spanned by its enumerators rather than the enumerators themselves. It is still not a valid encoding, and the same helper rejects it. Assisted-By: Claude Code (Claude Opus 5) * Validate the picture type byte before casting it The picture type enumeration is declared by DECLARE_PICTURE_TYPE_ENUM and shared by three classes, and all three cast a file-supplied value to it without checking. The enumerators run 0x00..0x14, so the range of the enumeration is 0..31 and a load outside that is undefined: taglib/mpeg/id3v2/frames/attachedpictureframe.cpp:96:13: runtime error: load of value 4294967295, which is not a valid value for type 'AttachedPictureFrame::Type' taglib/flac/flacpicture.cpp:129:13: runtime error: load of value 65536, which is not a valid value for type 'Type' FLAC is the widest of the three: it casts a whole 32 bit field, so no truncation to a byte limits it. ASF and ID3v2 read a plain signed char, which reaches the same place through sign extension. This is visible to callers, not only to a sanitizer. Before the change AttachedPictureFrame::type() and FLAC::Picture::type() return -1 and 65536 for the files above; after it they return Other. Since the enumeration comes from a macro, add one typeFromByte() to the macro backed by Utils::pictureTypeFromByte(), rather than three copies of the same check. Six reports before the change, none after. Assisted-By: Claude Code (Claude Opus 5) * ID3v2: validate the RVA2 channel byte before casting it Each channel record in a relative volume frame starts with a channel type byte that is cast straight to ChannelType. The enumerators run 0x00..0x08, so the range of the enumeration is 0..15 and the byte from the file can leave it: runtime error: load of value 127, which is not a valid value for type 'RelativeVolumeFrame::ChannelType' runtime error: load of value 4294967295, which is not a valid value for type 'RelativeVolumeFrame::ChannelType' The value is also used as a map key, so the frame ends up holding a channel that channels() then reports back. Map an unrecognised byte to Other instead. Two reports before the change, none after. The 123 files in tests/data produce identical channel output either way. Assisted-By: Claude Code (Claude Opus 5) * ID3v2: validate the SYLT and ETCO enum bytes before casting them Synchronised lyrics carry a timestamp format byte and a content type byte, and event timing codes carry a timestamp format byte. All three are cast without checking. TimestampFormat enumerates 0..2, so its range is only 0..3: runtime error: load of value 127, which is not a valid value for type 'SynchronizedLyricsFrame::TimestampFormat' runtime error: load of value 127, which is not a valid value for type 'SynchronizedLyricsFrame::Type' runtime error: load of value 127, which is not a valid value for type 'EventTimingCodesFrame::TimestampFormat' with 4294967295 in place of 127 when the byte is 0xFF. Before the change timestampFormat() and type() return -1 for such a file. Map an unrecognised byte to Unknown and Other respectively. Worth noting what is *not* changed: the event type byte on the line below the ETCO cast is already written static_cast<EventType>(static_cast<unsigned char>(...)), and EventType enumerates up to 0xFE, so an unsigned char cannot leave its range. It reads like the same defect and is not one. Three reports before the change, none after. The 123 files in tests/data produce identical output either way. Assisted-By: Claude Code (Claude Opus 5) * MP4: validate the atom data type before casting it The type field of an iTunes metadata atom is a 32 bit value read from the file and cast straight to AtomDataType, whose enumerators run 0..255. Values above 255 are outside the range of the enumeration: taglib/mp4/mp4atom.h:85:36: runtime error: load of value 65536, which is not a valid value for type 'AtomDataType' taglib/mp4/mp4atom.h:85:36: runtime error: load of value 4294967295, which is not a valid value for type 'AtomDataType' Both cast sites are reachable: parseFreeForm calls parseData2 with expectedFlags = -1, so the flags == expectedFlags test never constrains the value, and the mean/name branch casts unconditionally. A '----' atom with an arbitrary flags field reaches both. Map anything outside the range to TypeUndefined, which the enumeration already provides for exactly this. Four reports before the change, none after. The 123 files in tests/data produce identical item output either way. Assisted-By: Claude Code (Claude Opus 5)