[PATCH] Sanity check for ID3v2 extended header size

[lazka]

Original issue 126 created by aurelian@streber24.de on 2009-02-11T19:13:56.000Z:

I noticed that quodlibet would not read some tags from MP3 files for
seemingly no reason. Digging into the issue I found that my tagging program
sets the "extended header" flag (0x40) but does not actually write an
extended header. The first field name ("TRCK") gets interpreted as header
size, which causes EOFError if it is larger than the file size.

Of course such a file is obviously corrupted. However the tag is perfectly
readable if the condition is detected.

Suggested fix: Check the extended header size. If it exceeds a certain
limit, assume a corrupted file and clear the "extended header" flag instead
of trying to read the EH.

The attached patch does that. I set the limit to an arbitrary 0xFFFF
(65536) bytes.

[lazka]

Comment #1 originally posted by aurelian@streber24.de on 2009-02-12T16:41:32.000Z:

bugfix in the patch: seek backwards after reading corrupted header size (which is in
fact the first frame header).

[lazka]

Comment #2 originally posted by murman on 2009-02-14T04:41:18.000Z:

Instead of an arbitrary size, perhaps compare against the size of the tag, or the
size of the file, or take a heuristic approach like is done for the non/synch-safe
sizes miswritten by iTunes? Either way, this will require tests before it's
committed.

[lazka]

Comment #3 originally posted by aurelian@streber24.de on 2009-02-15T21:29:09.000Z:

You are right about the size. To me the most sensible thing seems to check against
the total tag size (so, self.size instead of 0xFFFF). Well, and for v<=2.3.0 the
extsize should be 6 or 10 anyways. IMHO a sophisticated logic would be overkill.

I can prepare another patch if wanted.

Oh, and test to your heart's content :-)

[lazka]

Comment #4 originally posted by steven.strobe.cc on 2009-03-16T20:34:01.000Z:

That's 'test' as in trunk/mutagen/tests/test_id3.py .

And, yes, I'm inventing another new tag in the issue tracker to describe this
situation. ;)

[lazka]

Comment #5 originally posted by steven.strobe.cc on 2009-06-17T21:16:26.000Z:

Moved to Mutagen.

http://code.google.com/p/mutagen/issues/detail?id=4