diff mbox

[FFmpeg-devel] libavcodec/mpegaudiodecheader.h : detect reserved mpeg id

Message ID 20180708102610.41201-1-ottoka@posteo.de
State Accepted
Commit 3bf39f2aeff0defcc11454c497b6ea0ffbcd17ca
Headers show

Commit Message

Karsten Otto July 8, 2018, 10:26 a.m. UTC
Check the MPEG version ID for the reserved bit pattern 01, and abort the
header check in that case. This reduces the chance of misinterpreting
arbitrary data as a valid header, and prevents resulting audio artifacts.
---
 libavcodec/mpegaudiodecheader.h | 3 +++
 1 file changed, 3 insertions(+)

Comments

Michael Niedermayer July 9, 2018, 12:46 a.m. UTC | #1
On Sun, Jul 08, 2018 at 12:26:10PM +0200, Karsten Otto wrote:
> Check the MPEG version ID for the reserved bit pattern 01, and abort the
> header check in that case. This reduces the chance of misinterpreting
> arbitrary data as a valid header, and prevents resulting audio artifacts.
> ---
>  libavcodec/mpegaudiodecheader.h | 3 +++
>  1 file changed, 3 insertions(+)

will apply

thx

[...]
Karsten Otto July 11, 2018, 10:03 p.m. UTC | #2
Argh, just noticed a stupid mistake - quite obviously, the check pattern
needs to be shifted too, i.e. 1<<19 instead of just 1. Luckily, it won't do
any harm, since it can never evaluate to true.

What is the protocol in this case - Do I send a new version of the patch?
Or another patch to the patch that is already applied?

Sorry for the inconvenience,
Karsten

> Am 09.07.2018 um 02:46 schrieb Michael Niedermayer <michael@niedermayer.cc>:
> 
> Signierter PGP-Teil
> On Sun, Jul 08, 2018 at 12:26:10PM +0200, Karsten Otto wrote:
>> Check the MPEG version ID for the reserved bit pattern 01, and abort the
>> header check in that case. This reduces the chance of misinterpreting
>> arbitrary data as a valid header, and prevents resulting audio artifacts.
>> ---
>> libavcodec/mpegaudiodecheader.h | 3 +++
>> 1 file changed, 3 insertions(+)
> 
> will apply
> 
> thx
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> When the tyrant has disposed of foreign enemies by conquest or treaty, and
> there is nothing more to fear from them, then he is always stirring up
> some war or other, in order that the people may require a leader. -- Plato
> 
>
Carl Eugen Hoyos July 11, 2018, 10:33 p.m. UTC | #3
2018-07-12 0:03 GMT+02:00, Karsten Otto <ottoka@posteo.de>:
> Argh, just noticed a stupid mistake - quite obviously, the check pattern
> needs to be shifted too, i.e. 1<<19 instead of just 1. Luckily, it won't do
> any harm, since it can never evaluate to true.
>
> What is the protocol in this case - Do I send a new version of the patch?
> Or another patch to the patch that is already applied?

It has to be a patch that can be committed, so in this case a patch
on top of the applied patch.

Please avoid top-posting here, Carl Eugen
diff mbox

Patch

diff --git a/libavcodec/mpegaudiodecheader.h b/libavcodec/mpegaudiodecheader.h
index 1cb9216461..ed9961250a 100644
--- a/libavcodec/mpegaudiodecheader.h
+++ b/libavcodec/mpegaudiodecheader.h
@@ -62,6 +62,9 @@  static inline int ff_mpa_check_header(uint32_t header){
     /* header */
     if ((header & 0xffe00000) != 0xffe00000)
         return -1;
+    /* version check */
+    if ((header & (3<<19)) == 1)
+        return -1;
     /* layer check */
     if ((header & (3<<17)) == 0)
         return -1;