Validate enumerations decoded from file bytes before casting to them (#1420)

* 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)

taglib/asf/asfpicture.cpp

@@ -133,7 +133,7 @@ void ASF::Picture::parse(const ByteVector& bytes)
   if(bytes.size() < 9)
     return;
   int pos = 0;
-  d->type = static_cast<Type>(bytes[0]); ++pos;
+  d->type = typeFromByte(bytes[0]); ++pos;
   const unsigned int dataLen = bytes.toUInt(pos, false); pos+=4;
 
   const ByteVector nullStringTerminator(2, 0);

taglib/flac/flacpicture.cpp

@@ -68,7 +68,7 @@ bool FLAC::Picture::parse(const ByteVector &data)
   }
 
   unsigned int pos = 0;
-  d->type = static_cast<FLAC::Picture::Type>(data.toUInt(pos));
+  d->type = typeFromUInt(data.toUInt(pos));
   pos += 4;
   unsigned int mimeTypeLength = data.toUInt(pos);
   pos += 4;

taglib/mp4/mp4itemfactory.cpp

@@ -41,6 +41,21 @@ namespace {
 
 constexpr char freeFormPrefix[] = "----:com.apple.iTunes:";
 
+/*!
+ * Returns the atom data type denoted by \a flags, the type field of an
+ * iTunes metadata atom, or TypeUndefined if it cannot be represented.
+ *
+ * The field is 32 bits wide and comes from the file, while the range of the
+ * enumeration is 0..255, so it cannot simply be cast.
+ */
+MP4::AtomDataType atomDataTypeFromFlags(int flags)
+{
+  if(flags >= MP4::TypeImplicit && flags <= MP4::TypeUndefined)
+    return static_cast<MP4::AtomDataType>(flags);
+
+  return MP4::TypeUndefined;
+}
+
 MP4::CoverArt::Format detectImageFormat(const ByteVector &payload)
 {
   const unsigned int size = payload.size();
@@ -452,7 +467,7 @@ MP4::AtomDataList ItemFactory::parseData2(
         debug("MP4: Unexpected atom \"" + name + "\", expecting \"name\"");
         return result;
       }
-      result.append(AtomData(static_cast<AtomDataType>(flags),
+      result.append(AtomData(atomDataTypeFromFlags(flags),
                     data.mid(pos + 12, length - 12)));
     }
     else {
@@ -461,7 +476,7 @@ MP4::AtomDataList ItemFactory::parseData2(
         return result;
       }
       if(expectedFlags == -1 || flags == expectedFlags) {
-        result.append(AtomData(static_cast<AtomDataType>(flags),
+        result.append(AtomData(atomDataTypeFromFlags(flags),
                       data.mid(pos + 16, length - 16)));
       }
     }

taglib/mpeg/id3v2/frames/attachedpictureframe.cpp

@@ -26,6 +26,7 @@
 #include "attachedpictureframe.h"
 
 #include "tstringlist.h"
+#include "tutils.h"
 #include "tdebug.h"
 
 using namespace TagLib;
@@ -132,7 +133,7 @@ void AttachedPictureFrame::parseFields(const ByteVector &data)
     return;
   }
 
-  d->textEncoding = static_cast<String::Type>(data[0]);
+  d->textEncoding = Utils::textEncodingFromByte(data[0]);
 
   int pos = 1;
 
@@ -143,7 +144,7 @@ void AttachedPictureFrame::parseFields(const ByteVector &data)
     return;
   }
 
-  d->type = static_cast<TagLib::ID3v2::AttachedPictureFrame::Type>(data[pos++]);
+  d->type = typeFromByte(data[pos++]);
   d->description = readStringField(data, d->textEncoding, &pos);
 
   d->data = data.mid(pos);
@@ -188,7 +189,7 @@ void AttachedPictureFrameV22::parseFields(const ByteVector &data)
     return;
   }
 
-  d->textEncoding = static_cast<String::Type>(data[0]);
+  d->textEncoding = Utils::textEncodingFromByte(data[0]);
 
   int pos = 1;
 
@@ -204,7 +205,7 @@ void AttachedPictureFrameV22::parseFields(const ByteVector &data)
     d->mimeType = "image/" + fixedString;
   }
 
-  d->type = static_cast<TagLib::ID3v2::AttachedPictureFrame::Type>(data[pos++]);
+  d->type = typeFromByte(data[pos++]);
   d->description = readStringField(data, d->textEncoding, &pos);
 
   d->data = data.mid(pos);

taglib/mpeg/id3v2/frames/commentsframe.cpp

@@ -28,6 +28,7 @@
 #include <utility>
 
 #include "tbytevectorlist.h"
+#include "tutils.h"
 #include "tdebug.h"
 #include "tstringlist.h"
 #include "tpropertymap.h"
@@ -143,7 +144,7 @@ void CommentsFrame::parseFields(const ByteVector &data)
     return;
   }
 
-  d->textEncoding = static_cast<String::Type>(data[0]);
+  d->textEncoding = Utils::textEncodingFromByte(data[0]);
   d->language = data.mid(1, 3);
 
   int byteAlign = d->textEncoding == String::Latin1 || d->textEncoding == String::UTF8 ? 1 : 2;

taglib/mpeg/id3v2/frames/eventtimingcodesframe.cpp

@@ -34,6 +34,23 @@
 using namespace TagLib;
 using namespace ID3v2;
 
+namespace
+{
+  /*!
+   * Returns the timestamp format denoted by \a c, or Unknown if it denotes
+   * nothing. The range of the enumeration is only 0..3, so the byte read from
+   * the file cannot simply be cast.
+   */
+  EventTimingCodesFrame::TimestampFormat timestampFormatFromByte(char c)
+  {
+    if(const auto value = static_cast<unsigned char>(c);
+       value <= EventTimingCodesFrame::AbsoluteMilliseconds)
+      return static_cast<EventTimingCodesFrame::TimestampFormat>(value);
+
+    return EventTimingCodesFrame::Unknown;
+  }
+} // namespace
+
 class EventTimingCodesFrame::EventTimingCodesFramePrivate
 {
 public:
@@ -101,7 +118,7 @@ void EventTimingCodesFrame::parseFields(const ByteVector &data)
     return;
   }
 
-  d->timestampFormat = static_cast<TimestampFormat>(data[0]);
+  d->timestampFormat = timestampFormatFromByte(data[0]);
 
   int pos = 1;
   d->synchedEvents.clear();

taglib/mpeg/id3v2/frames/generalencapsulatedobjectframe.cpp

@@ -28,6 +28,7 @@
 
 #include "generalencapsulatedobjectframe.h"
 
+#include "tutils.h"
 #include "tdebug.h"
 #include "tstringlist.h"
 
@@ -142,7 +143,7 @@ void GeneralEncapsulatedObjectFrame::parseFields(const ByteVector &data)
     return;
   }
 
-  d->textEncoding = static_cast<String::Type>(data[0]);
+  d->textEncoding = Utils::textEncodingFromByte(data[0]);
 
   int pos = 1;
 

taglib/mpeg/id3v2/frames/ownershipframe.cpp

@@ -25,6 +25,7 @@
 
 #include "ownershipframe.h"
 
+#include "tutils.h"
 #include "tstringlist.h"
 #include "id3v2tag.h"
 
@@ -124,7 +125,7 @@ void OwnershipFrame::parseFields(const ByteVector &data)
   }
 
   // Get the text encoding
-  d->textEncoding = static_cast<String::Type>(data[0]);
+  d->textEncoding = Utils::textEncodingFromByte(data[0]);
   pos += 1;
 
   // Read the price paid, this is a null terminated string

taglib/mpeg/id3v2/frames/relativevolumeframe.cpp

@@ -32,6 +32,25 @@
 using namespace TagLib;
 using namespace ID3v2;
 
+namespace
+{
+  /*!
+   * Returns the channel type denoted by \a c, the channel byte of an RVA2
+   * channel record, or Other if it denotes nothing.
+   *
+   * The byte comes from the file, and the range of the enumeration is only
+   * 0..15, so the value cannot simply be cast.
+   */
+  RelativeVolumeFrame::ChannelType channelTypeFromByte(char c)
+  {
+    if(const auto value = static_cast<unsigned char>(c);
+       value <= RelativeVolumeFrame::Subwoofer)
+      return static_cast<RelativeVolumeFrame::ChannelType>(value);
+
+    return RelativeVolumeFrame::Other;
+  }
+} // namespace
+
 struct ChannelData
 {
   RelativeVolumeFrame::ChannelType channelType { RelativeVolumeFrame::Other };
@@ -133,7 +152,7 @@ void RelativeVolumeFrame::parseFields(const ByteVector &data)
 
   while(pos <= static_cast<int>(data.size()) - 4) {
 
-    auto type = static_cast<ChannelType>(data[pos]);
+    auto type = channelTypeFromByte(data[pos]);
     pos += 1;
 
     ChannelData &channel = d->channels[type];

taglib/mpeg/id3v2/frames/synchronizedlyricsframe.cpp

@@ -28,6 +28,7 @@
 #include <utility>
 
 #include "tbytevectorlist.h"
+#include "tutils.h"
 #include "tdebug.h"
 #include "tpropertymap.h"
 #include "id3v2tag.h"
@@ -35,6 +36,36 @@
 using namespace TagLib;
 using namespace ID3v2;
 
+namespace
+{
+  /*!
+   * Returns the timestamp format denoted by \a c, or Unknown if it denotes
+   * nothing. The range of the enumeration is only 0..3, so the byte read from
+   * the file cannot simply be cast.
+   */
+  SynchronizedLyricsFrame::TimestampFormat timestampFormatFromByte(char c)
+  {
+    if(const auto value = static_cast<unsigned char>(c);
+       value <= SynchronizedLyricsFrame::AbsoluteMilliseconds)
+      return static_cast<SynchronizedLyricsFrame::TimestampFormat>(value);
+
+    return SynchronizedLyricsFrame::Unknown;
+  }
+
+  /*!
+   * Returns the content type denoted by \a c, or Other if it denotes nothing.
+   * The range of the enumeration is only 0..15.
+   */
+  SynchronizedLyricsFrame::Type contentTypeFromByte(char c)
+  {
+    if(const auto value = static_cast<unsigned char>(c);
+       value <= SynchronizedLyricsFrame::ImageUrls)
+      return static_cast<SynchronizedLyricsFrame::Type>(value);
+
+    return SynchronizedLyricsFrame::Other;
+  }
+} // namespace
+
 class SynchronizedLyricsFrame::SynchronizedLyricsFramePrivate
 {
 public:
@@ -146,10 +177,10 @@ void SynchronizedLyricsFrame::parseFields(const ByteVector &data)
     return;
   }
 
-  d->textEncoding = static_cast<String::Type>(data[0]);
+  d->textEncoding = Utils::textEncodingFromByte(data[0]);
   d->language = data.mid(1, 3);
-  d->timestampFormat = static_cast<TimestampFormat>(data[4]);
-  d->type = static_cast<Type>(data[5]);
+  d->timestampFormat = timestampFormatFromByte(data[4]);
+  d->type = contentTypeFromByte(data[5]);
 
   int pos = 6;
 

taglib/mpeg/id3v2/frames/textidentificationframe.cpp

@@ -29,6 +29,7 @@
 #include <array>
 #include <utility>
 
+#include "tutils.h"
 #include "tpropertymap.h"
 #include "id3v1genres.h"
 #include "id3v2tag.h"
@@ -218,7 +219,7 @@ void TextIdentificationFrame::parseFields(const ByteVector &data)
 
   // read the string data type (the first byte of the field data)
 
-  d->textEncoding = static_cast<String::Type>(data[0]);
+  d->textEncoding = Utils::textEncodingFromByte(data[0]);
 
   // split the byte array into chunks based on the string type (two byte delimiter
   // for unicode encodings)

taglib/mpeg/id3v2/frames/unsynchronizedlyricsframe.cpp

@@ -31,6 +31,7 @@
 #include <utility>
 
 #include "tbytevectorlist.h"
+#include "tutils.h"
 #include "tdebug.h"
 #include "tpropertymap.h"
 #include "id3v2tag.h"
@@ -144,7 +145,7 @@ void UnsynchronizedLyricsFrame::parseFields(const ByteVector &data)
     return;
   }
 
-  d->textEncoding = static_cast<String::Type>(data[0]);
+  d->textEncoding = Utils::textEncodingFromByte(data[0]);
   d->language = data.mid(1, 3);
 
   int byteAlign

taglib/mpeg/id3v2/frames/urllinkframe.cpp

@@ -30,6 +30,7 @@
 
 #include <utility>
 
+#include "tutils.h"
 #include "tdebug.h"
 #include "tstringlist.h"
 #include "tpropertymap.h"
@@ -196,7 +197,7 @@ void UserUrlLinkFrame::parseFields(const ByteVector &data)
 
   int pos = 0;
 
-  d->textEncoding = static_cast<String::Type>(data[0]);
+  d->textEncoding = Utils::textEncodingFromByte(data[0]);
   pos += 1;
 
   if(d->textEncoding == String::Latin1 || d->textEncoding == String::UTF8) {

taglib/mpeg/id3v2/id3v2framefactory.cpp

@@ -28,6 +28,7 @@
 #include <array>
 #include <utility>
 
+#include "tutils.h"
 #include "tdebug.h"
 #include "tzlib.h"
 #include "id3v2synchdata.h"
@@ -373,13 +374,13 @@ void FrameFactory::rebuildAggregateFrames(ID3v2::Tag *tag) const
        tdat &&
        tdat->data().size() >= 5)
     {
-      String date(tdat->data().mid(1), static_cast<String::Type>(tdat->data()[0]));
+      String date(tdat->data().mid(1), Utils::textEncodingFromByte(tdat->data()[0]));
       if(date.length() == 4) {
         tdrc->setText(tdrc->toString() + '-' + date.substr(2, 2) + '-' + date.substr(0, 2));
         if(tag->frameList("TIME").size() == 1) {
           auto timeframe = dynamic_cast<UnknownFrame *>(tag->frameList("TIME").front());
           if(timeframe && timeframe->data().size() >= 5) {
-            String time(timeframe->data().mid(1), static_cast<String::Type>(timeframe->data()[0]));
+            String time(timeframe->data().mid(1), Utils::textEncodingFromByte(timeframe->data()[0]));
             if(time.length() == 4) {
               tdrc->setText(tdrc->toString() + 'T' + time.substr(0, 2) + ':' + time.substr(2, 2));
             }

taglib/toolkit/tpicturetype.cpp

@@ -74,3 +74,11 @@ int Utils::pictureTypeFromString(const String& str)
   }
   return 0;
 }
+
+int Utils::pictureTypeFromUInt(unsigned int value)
+{
+  if(value < std::size(typeStrs))
+    return static_cast<int>(value);
+
+  return 0;
+}

taglib/toolkit/tpicturetype.h

@@ -96,6 +96,15 @@ static TagLib::String typeToString(name type) {               \
 static name typeFromString(const TagLib::String &str) {       \
   return static_cast<name>(                                   \
     TagLib::Utils::pictureTypeFromString(str));               \
+}                                                             \
+static name typeFromUInt(unsigned int value) {                \
+  return static_cast<name>(                                   \
+    TagLib::Utils::pictureTypeFromUInt(value));               \
+}                                                             \
+static name typeFromByte(char value) {                        \
+  return static_cast<name>(                                   \
+    TagLib::Utils::pictureTypeFromUInt(                       \
+      static_cast<unsigned char>(value)));                    \
 }
 
 namespace TagLib {
@@ -114,6 +123,16 @@ namespace TagLib {
      */
     int TAGLIB_EXPORT pictureTypeFromString(const String& str);
 
+    /*!
+     * Get picture type from the \a value read from a file, or Other if it
+     * denotes no picture type.
+     *
+     * The value cannot simply be cast: loading an enumeration object whose
+     * value is outside the range of the enumeration is undefined, and the
+     * range here is only 0..31.
+     */
+    int TAGLIB_EXPORT pictureTypeFromUInt(unsigned int value);
+
   }  // namespace Utils
 }  // namespace TagLib
 

taglib/toolkit/tutils.h

@@ -58,6 +58,29 @@ namespace TagLib
     namespace
     {
 
+      /*!
+       * Returns the String::Type denoted by \a c, the text encoding byte at the
+       * start of an ID3v2 frame, or String::Latin1 if it denotes nothing.
+       *
+       * The byte comes from the file, so it cannot simply be cast: loading an
+       * enumeration object whose value is outside the enumeration is undefined, and
+       * String::data() switches on the type without a default case. Latin1 is what
+       * the frames already declare as their default encoding.
+       */
+      inline String::Type textEncodingFromByte(char c)
+      {
+        switch(static_cast<unsigned char>(c)) {
+        case String::Latin1:
+        case String::UTF16:
+        case String::UTF16BE:
+        case String::UTF8:
+        case String::UTF16LE:
+          return static_cast<String::Type>(static_cast<unsigned char>(c));
+        default:
+          return String::Latin1;
+        }
+      }
+
       /*!
        * Reverses the order of bytes in a 16-bit integer.
        */