Am 12.04.2011 um 16:51 schrieb Kostya: > On Tue, Apr 12, 2011 at 04:34:19PM +0200, Max Horn wrote: >> Hi again! >> >> Am 12.04.2011 um 15:59 schrieb Kostya: >> >>> On Tue, Apr 12, 2011 at 03:35:04PM +0200, Max Horn wrote: > [...]
>> I guess this means I should report similar and worse things in all the other >> parts of libav code? Like e.g. ff_get_wav_header, which is used by several >> demuxers, and which has no checks against invalid cbData, and does not check >> whether av_mallocz returns NULL? This looks like a much easier to exploit >> code spot to me... :) > > It's a good place to remind our motto: "Patch is welcome" > (yes, not checking alloc result is bad) It's just that it feels a bit unfair to be upheld to standards that seem a much higher than what a lot of the existing code meets... I mean, it took me about 2 minutes to come to find a piece of code in libav that has worse security issues than my patch ever had... Hum... Anyway, I absolutely do see your point about not knowingly introducing new potentially problematic code -- which is why I didn't hesitate to address it with my updated patch. :) And I would happily write a patch for riff.c and ff_get_wav_header in. Unfortunately, ff_get_wav_header has no way to signal an error to the caller. So that leaves the options of inserting (the equivalent of) an assert(); or to change the signature of ff_get_wav_header, e.g. by adding a return value that can be used to signal an error. In the latter case, all calling code (seven demuxers, including xwma) would have to be adjusted, too. [...] > >> </rant> >> >> OK, enough unconstructive ranting :). I know how it is with software growing >> over years and stuff. So back to more useful things: Attached is a revised >> patch hopefully addressing Kostya's concerns. Thank you very much for your >> feedback! > > I remind you that there's also Diego who can complain about wrongly worded > comments, formatting style and such, but in general patch looks good for me. I perfectly understand that multiple people on this project may have to review a patch before it gets accepted. And that it can take some days or longer, too. So I'll just wait. Thanks again for your feedback, Max _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
