diff mbox

[FFmpeg-devel] lafv/wavdec: Fail bext parsing on incomplete reads

Message ID CAADho6O8vcqSFZZrtxY=B1xwGKhD6cN=KOV+K2L8EuDfs3tRyw@mail.gmail.com
State New
Headers show

Commit Message

Matthew Wolenetz July 25, 2019, 11:09 p.m. UTC

Comments

Michael Niedermayer July 26, 2019, 6:37 a.m. UTC | #1 
On Thu, Jul 25, 2019 at 04:09:35PM -0700, Matthew Wolenetz wrote:
> 

>  wavdec.c |   12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 3e8d230a42a4a12aaf1c375f5a064924238992f9  0001-lafv-wavdec-Fail-bext-parsing-on-incomplete-reads.patch
> From 7966786250d9581891e0859f769a63f35a5c2729 Mon Sep 17 00:00:00 2001
> From: Matt Wolenetz <wolenetz@google.com>
> Date: Thu, 25 Jul 2019 15:54:49 -0700
> Subject: [PATCH] lafv/wavdec: Fail bext parsing on incomplete reads
> 
> avio_read can successfully return even when less than the requested
> amount of input was read. wavdec's bext parsing mistakenly assumed a
> successful avio_read always read the full amount that was requested.
> The result could be dictionary tags populated with partially
> uninitialized values.
> 
> This change also fixes a broken assertion in wav_parse_bext_string that
> was off-by-one, though no known current usage of that method hits that
> broken case.
> 
> Chromium bug: 987270
> 
> Signed-off-by: Matt Wolenetz <wolenetz@chromium.org>
> ---
>  libavformat/wavdec.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)

will apply

thanks

[...]
diff mbox

Patch

From 7966786250d9581891e0859f769a63f35a5c2729 Mon Sep 17 00:00:00 2001
From: Matt Wolenetz <wolenetz@google.com>
Date: Thu, 25 Jul 2019 15:54:49 -0700
Subject: [PATCH] lafv/wavdec: Fail bext parsing on incomplete reads

avio_read can successfully return even when less than the requested
amount of input was read. wavdec's bext parsing mistakenly assumed a
successful avio_read always read the full amount that was requested.
The result could be dictionary tags populated with partially
uninitialized values.

This change also fixes a broken assertion in wav_parse_bext_string that
was off-by-one, though no known current usage of that method hits that
broken case.

Chromium bug: 987270

Signed-off-by: Matt Wolenetz <wolenetz@chromium.org>
---
 libavformat/wavdec.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
index 1b131ee2c1..684efd97f9 100644
--- a/libavformat/wavdec.c
+++ b/libavformat/wavdec.c
@@ -233,9 +233,9 @@  static inline int wav_parse_bext_string(AVFormatContext *s, const char *key,
     char temp[257];
     int ret;
 
-    av_assert0(length <= sizeof(temp));
-    if ((ret = avio_read(s->pb, temp, length)) < 0)
-        return ret;
+    av_assert0(length < sizeof(temp));
+    if ((ret = avio_read(s->pb, temp, length)) != length)
+        return ret < 0 ? ret : AVERROR_INVALIDDATA;
 
     temp[length] = 0;
 
@@ -304,8 +304,10 @@  static int wav_parse_bext_tag(AVFormatContext *s, int64_t size)
         if (!(coding_history = av_malloc(size + 1)))
             return AVERROR(ENOMEM);
 
-        if ((ret = avio_read(s->pb, coding_history, size)) < 0)
-            return ret;
+        if ((ret = avio_read(s->pb, coding_history, size)) != size) {
+            av_free(coding_history);
+            return ret < 0 ? ret : AVERROR_INVALIDDATA;
+        }
 
         coding_history[size] = 0;
         if ((ret = av_dict_set(&s->metadata, "coding_history", coding_history,
-- 
2.22.0.709.g102302147b-goog