diff mbox series

[FFmpeg-devel,1/2] avformat/westwood_audenc: Check for, not assert on invalid data

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
Related show

Checks

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

Commit Message

Andreas Rheinhardt April 26, 2021, 1:01 p.m. UTC
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(-)

Comments

Aidan Richmond April 26, 2021, 2:42 p.m. UTC | #1
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 mbox series

Patch

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);