| Message ID | HE1PR0301MB21542C79A25302D34726BFB48F429@HE1PR0301MB2154.eurprd03.prod.outlook.com |
|---|---|
| State | Accepted |
| Commit | a64d4de0d44a07bcea49f24c2a8a972954e81d0a |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avformat/westwood_audenc: Check for, not assert on invalid data | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On 26/04/2021 14:01, Andreas Rheinhardt wrote: > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > Is pkt->size * 4 actually supposed to be the size of audio after > decoding? If so, the factor four would have to be changed to two > for files flagged as 8 bit. > (The 8/16 bit check seems broken; my actual intention with not > unconditionally flagging the file as 16 bit was that remuxing content > flagged as 8 bit should work, but it doesn't, because the current check > only checks for the codec_id.) > > libavformat/westwood_audenc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/westwood_audenc.c b/libavformat/westwood_audenc.c > index 4ec905b088..490f2ee260 100644 > --- a/libavformat/westwood_audenc.c > +++ b/libavformat/westwood_audenc.c > @@ -103,7 +103,8 @@ static int wsaud_write_packet(AVFormatContext *ctx, AVPacket *pkt) > AVIOContext *pb = ctx->pb; > AUDMuxContext *a = ctx->priv_data; > > - av_assert1(pkt->size < UINT16_MAX && (pkt->size * 4) < UINT16_MAX); > + if (pkt->size > UINT16_MAX / 4) > + return AVERROR_INVALIDDATA; > /* Assumes ADPCM since this muxer doesn't support SND1 or PCM format. */ > avio_wl16(pb, pkt->size); > avio_wl16(pb, pkt->size * 4); > You are right, I didn't get the check quite right, I actually struggled to get a field that would provide the correct information as the suggested bits_per_raw_sample contained 0 when I tested it, resulting in incorrectly flagged files. I wasn't really sure how else to solve it and in hindsight the check I added didn't really do anything. The demuxer also appears to suffer from the same limitation as it ignores checking the flag in the header for 16bit and just assumes it is and the decoder as well as the new encoder both also assume they are handling 16bit samples.
diff --git a/libavformat/westwood_audenc.c b/libavformat/westwood_audenc.c index 4ec905b088..490f2ee260 100644 --- a/libavformat/westwood_audenc.c +++ b/libavformat/westwood_audenc.c @@ -103,7 +103,8 @@ static int wsaud_write_packet(AVFormatContext *ctx, AVPacket *pkt) AVIOContext *pb = ctx->pb; AUDMuxContext *a = ctx->priv_data; - av_assert1(pkt->size < UINT16_MAX && (pkt->size * 4) < UINT16_MAX); + if (pkt->size > UINT16_MAX / 4) + return AVERROR_INVALIDDATA; /* Assumes ADPCM since this muxer doesn't support SND1 or PCM format. */ avio_wl16(pb, pkt->size); avio_wl16(pb, pkt->size * 4);
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- Is pkt->size * 4 actually supposed to be the size of audio after decoding? If so, the factor four would have to be changed to two for files flagged as 8 bit. (The 8/16 bit check seems broken; my actual intention with not unconditionally flagging the file as 16 bit was that remuxing content flagged as 8 bit should work, but it doesn't, because the current check only checks for the codec_id.) libavformat/westwood_audenc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)