Quoting David Bryant (2020-04-06 06:11:17) > On 4/2/20 11:32 PM, Anton Khirnov wrote: > > > >>> Also, I looked at the specification PDF you linked in the previous email > >>> and I see nothing in it that explicitly forbids the channel count from > >>> changing from one "superblock" to the next. It seems to me like you > >>> could just concatenate wavpack files with different channel counts and > >>> get a valid wavpack file. > >>> > >> You're right, I could not find that being explicitly forbidden. > >> Nevertheless, it _is_ forbidden for any stream parameters to > >> change (e.g. data format, sampling rate, channel count or mapping) and > >> both libwavpack and the FFmpeg demuxer enforce this (see > >> libavformat/wvdec.c starting at line 211), or at least gracefully fail > >> without crashing. > > Forbidden by whom? From what I can see: > > - sample format: the wv demuxer does not set it at all, the decoder sets > > it in wavpack_decode_frame() from the block header; also, Michael sent > > a patch recently to deal with a wavpack decoder crash related to > > format changes > > - sample rate: set by wv demuxer and then checked that it does not > > change; but the caller is not necessarily using the wv demuxer and the > > decoder will happily change it the same way it does with channel count > > > >> The reason it's never mentioned is probably just because it never occurred > >> to me. None of the source file formats for WavPack > >> (e.g., wav, caf, dsdiff) allow format changes and neither do any of the > >> other lossless compressors I am aware of. > > Correct, I cannot see any other lossless codecs that would allow this. > > But then again all these codecs have global headers. So it's not that > > they artificially forbid format changes, but the bistream format is not > > able to represent them. > > I disagree with that. It would be trivial to add a new STREAMINFO block to a > FLAC file and change the format after that. This is > not specifically prohibited in the FLAC specs, but it's sort of implied when > it says that the initial STREAMINFO block has > "information about the whole stream".
From a quick glance at the spec, metadata blocks are only allowed at the beginning and are followed by audio frames. Once you see a single audio frame, there should be no further metadata blocks. That said, the frame header does contain stream parameters and those can conceivably change and after looking at our decoder, it does support those changes. It was added explicitly in commit 90fcac0e95b7d266c148a86506f301a2072d9de3 Author: Justin Ruggles <justin.rugg...@gmail.com> Date: Sun Oct 21 17:02:28 2012 -0400 flacdec: allow mid-stream channel layout change Although the libFLAC decoder cannot handle such a change, it is allowed by the spec and could potentially occur with live streams. > > > > >> and extensive fuzzing has ensured that pathological files like you > >> suggest are unlikely to be the source of exploits. > > See the crash above. I think fuzzing is very overhyped these days. It's > > a useful tool for sure, but one should not rely purely on fuzzing too > > much. It may not cover important parts of the input space because of the > > way it is set up. Or there could be exploitable codepaths that require > > too specific inputs for a random process to find, but a determined > > attacker could construct them from reading the code. > > > Of course fuzzing cannot find everything, but my experience has been that > fuzzing finds things that I would _never_ find by just > looking at the code. > > And I was certainly not suggesting that one should just write sloppy code > knowing that all the bugs will be found with fuzzing. Yeah, I certainly agree that fuzzing is a very useful tool. My point is just that it shouldn't be your _only_ tool. There can be plenty of bugs fuzzing won't find. -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".