[FFmpeg-devel] [PATCH 5/9] nistspheredec: prevent overflow during block alignment calculation

[Andreas Cadhalpun]

On 26.01.2017 02:29, Ronald S. Bultje wrote:
> On Wed, Jan 25, 2017 at 8:12 PM, Andreas Cadhalpun <
> andreas.cadhalpun at googlemail.com> wrote:
> 
>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>> ---
>>  libavformat/nistspheredec.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/libavformat/nistspheredec.c b/libavformat/nistspheredec.c
>> index 782d1dfbfb..3386497682 100644
>> --- a/libavformat/nistspheredec.c
>> +++ b/libavformat/nistspheredec.c
>> @@ -21,6 +21,7 @@
>>
>>  #include "libavutil/avstring.h"
>>  #include "libavutil/intreadwrite.h"
>> +#include "libavcodec/internal.h"
>>  #include "avformat.h"
>>  #include "internal.h"
>>  #include "pcm.h"
>> @@ -90,6 +91,11 @@ static int nist_read_header(AVFormatContext *s)
>>              return 0;
>>          } else if (!memcmp(buffer, "channel_count", 13)) {
>>              sscanf(buffer, "%*s %*s %"SCNd32, &st->codecpar->channels);
>> +            if (st->codecpar->channels > FF_SANE_NB_CHANNELS) {
>> +                av_log(s, AV_LOG_ERROR, "Too many channels %d > %d\n",
>> +                       st->codecpar->channels, FF_SANE_NB_CHANNELS);
>> +                return AVERROR(ENOSYS);
>> +            }
> 
> 
> I've said this before, but again - please don't add useless log messages.

I disagree that these log messages are useless and I'm not the only one [1].

> These don't help end users at all, since these samples are exceedingly
> unlikely to be real.

Files can get corrupted randomly, so I think this error isn't less likely
than most other errors.

> Most likely, they derive from fuzzing, and stderr
> cramming is one of the things that makes fuzzing slower.

Printing log messages in inner decoding loops makes that slower, but a
log message during header parsing is hardly noticeable.

Best regards,
Andreas


1: https://ffmpeg.org/pipermail/ffmpeg-devel/2017-January/205433.html