diff mbox series

[FFmpeg-devel,05/25] avfilter/af_headphone: Don't overrun array

Message ID 20200908211856.16290-5-andreas.rheinhardt@gmail.com
State Accepted
Commit 14226be499d27935d54981f0a6e1b15fd65746cd
Headers show
Series [FFmpeg-devel,01/25] avfilter/af_headphone: Don't use uninitialized buffer in log message | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Sept. 8, 2020, 9:18 p.m. UTC 
The headphone filter stores the channel position of the ith HRIR stream
in the ith element of an array of 64 elements; but because there is no
check for duplicate channels, it is easy to write beyond the end of the
array by simply repeating channels.

This commit adds a check for duplicate channels to rule this out.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavfilter/af_headphone.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

Paul B Mahol Sept. 9, 2020, 12:51 a.m. UTC | #1 
On Tue, Sep 08, 2020 at 11:18:36PM +0200, Andreas Rheinhardt wrote:
> The headphone filter stores the channel position of the ith HRIR stream
> in the ith element of an array of 64 elements; but because there is no
> check for duplicate channels, it is easy to write beyond the end of the
> array by simply repeating channels.
> 
> This commit adds a check for duplicate channels to rule this out.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavfilter/af_headphone.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 

LGTM
diff mbox series

Patch

diff --git a/libavfilter/af_headphone.c b/libavfilter/af_headphone.c
index 54b5dfec4c..99bdefbcff 100644
--- a/libavfilter/af_headphone.c
+++ b/libavfilter/af_headphone.c
@@ -88,15 +88,13 @@  typedef struct HeadphoneContext {
     } *in;
 } HeadphoneContext;
 
-static int parse_channel_name(HeadphoneContext *s, int x, char **arg, int *rchannel, char *buf)
+static int parse_channel_name(char **arg, int *rchannel, char *buf)
 {
     int len, i, channel_id = 0;
     int64_t layout, layout0;
 
     if (sscanf(*arg, "%7[A-Z]%n", buf, &len)) {
         layout0 = layout = av_get_channel_layout(buf);
-        if (layout == AV_CH_LOW_FREQUENCY)
-            s->lfe_channel = x;
         for (i = 32; i > 0; i >>= 1) {
             if (layout >= 1LL << i) {
                 channel_id += i;
@@ -116,6 +114,7 @@  static void parse_map(AVFilterContext *ctx)
 {
     HeadphoneContext *s = ctx->priv;
     char *arg, *tokenizer, *p, *args = av_strdup(s->map);
+    uint64_t used_channels = 0;
     int i;
 
     if (!args)
@@ -134,10 +133,17 @@  static void parse_map(AVFilterContext *ctx)
         char buf[8];
 
         p = NULL;
-        if (parse_channel_name(s, s->nb_irs, &arg, &out_ch_id, buf)) {
+        if (parse_channel_name(&arg, &out_ch_id, buf)) {
             av_log(ctx, AV_LOG_WARNING, "Failed to parse \'%s\' as channel name.\n", arg);
             continue;
         }
+        if (used_channels & (1ULL << out_ch_id)) {
+            av_log(ctx, AV_LOG_WARNING, "Ignoring duplicate channel '%s'.\n", buf);
+            continue;
+        }
+        used_channels |= 1ULL << out_ch_id;
+        if (out_ch_id == av_log2(AV_CH_LOW_FREQUENCY))
+            s->lfe_channel = s->nb_irs;
         s->mapping[s->nb_irs] = out_ch_id;
         s->nb_irs++;
     }