rexjw / mp4v2

Automatically exported from code.google.com/p/mp4v2
Other
0 stars 0 forks source link

Malformed files cause segmentation faults #125

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Run mp4info on the attached file

What is the expected output? What do you see instead?
I expect to see some information on the file, but instead it crashes:

/Volumes/Home/sbooth/Downloads/04 BBB.m4a:
ReadChildAtoms: "/Volumes/Home/sbooth/Downloads/04 BBB.m4a": In atom ?lyr 
missing child atom data
Track   Type    Info
1   audio   MPEG-4 AAC LC, 211.417 secs, 128 kbps, 44100 Hz
ReadChildAtoms: "/Volumes/Home/sbooth/Downloads/04 BBB.m4a": In atom ?lyr 
missing child atom data
Segmentation fault: 11

What version of the product are you using? On what operating system?
The latest svn version

Please provide any additional information below.
I've attached a patch which fixes the problem.

There is a comment in MP4ItmfItem_s that says that dataList.size is always >=1 
(itmf_generic.h:126) but in __itemAtomToModel (generic.cpp:154) if there are no 
data atoms the size is left at 0 (line 188).  This means that all the fetchXXX 
functions in Tag.cpp will segfault when f->second->dataList.elements[0] is 
called. The patch remedies the issue by verifying that dataList.size is at 
least 1 and returns if it isn't.  I don't know if this is strictly the desired 
behavior, as the comment indicates that size should always be at least one, but 
in my testing I didn't notice any side effects.

Original issue reported on code.google.com by stephen....@gmail.com on 30 Nov 2011 at 1:16

Attachments:

GoogleCodeExporter commented 9 years ago
Is someone available to look at this issue and commit the fix?

Original comment by stephen....@gmail.com on 12 Jan 2012 at 12:42

GoogleCodeExporter commented 9 years ago
Stephen,

Sorry, thanks for the reminder.  I'll have a look today hopefully; definitely 
this week.

Original comment by jnor...@logitech.com on 12 Jan 2012 at 5:43

GoogleCodeExporter commented 9 years ago
This should be fixed in r490.  Quick question: do you know if you can read the 
lyric tag with any other applications out there?  Or is this file somehow 
invalid?

I think you're correct--the app shouldn't segfault on the file--but I'm 
wondering if there's some improvement to be made to read this file in full, or 
if it's just bogus at some basic level.  Thanks for the report.

Original comment by kid...@gmail.com on 19 Mar 2012 at 2:57

GoogleCodeExporter commented 9 years ago
Issue 131 has been merged into this issue.

Original comment by kid...@gmail.com on 20 May 2012 at 8:05

GoogleCodeExporter commented 9 years ago
It sounds like the lyrics atom has no child atoms, so there is nothing to read. 
 (If I'm wrong about the file described above, this is definitely the case with 
the album atom in the file I dumped for #131, and I can easily create a file 
with a childless lyrics atom as well.) The only real question is whether to 
treat it the same as an empty lyrics atom, as if there were no lyrics atom at 
all, or as a lyrics atom with a special null value. iTunes isn't much of a 
guide here, because it just shows an empty lyrics field, indistinguishable from 
a file with no lyrics atom, or a lyrics atom with a subatom with an empty 
string (and if you edit the file to add some lyrics, it writes a new lyrics 
atom with a child with the new lyrics). I'll do a bit of playing around with 
the latest OS X and iOS versions of AVFoundation to see if they provide more of 
a guide. But I think what the library, as patched above, is doing now is 
probably fine.

Original comment by abarn...@gmail.com on 20 May 2012 at 9:53

GoogleCodeExporter commented 9 years ago
Thanks for the update, I appreciate it.

Original comment by kid...@gmail.com on 20 May 2012 at 9:54