diff mbox series

[FFmpeg-devel,1/2] avformat/mov: Fix extended atom size buffer length check

Message ID 20210318161456.1103652-1-derek.buitenhuis@gmail.com
State Accepted
Commit 85f397c828c8766d411d7bfc773c1241057e9d30
Headers show
Series [FFmpeg-devel,1/2] avformat/mov: Fix extended atom size buffer length check | expand

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

Derek Buitenhuis March 18, 2021, 4:14 p.m. UTC
When extended atom size support was added to probing in
fec4a2d232d7ebf6d1084fb568d4d84844f25abc, the buffer
size check was backwards, but probing continued to work
because there was no minimum size check yet, so despite
size being 1 on these atoms, and failing to read the 64-bit
size, the tag was still correctly read.

When 0b78016b2d7c36b32d07669c0c86bc4b4225ec98 introduced a
minimum size check, this exposed the bug, and broke probing
any files with extended atom sizes, such as entirely valid
large files that start whith mdat atoms.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 libavformat/mov.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Derek Buitenhuis March 21, 2021, 2:44 p.m. UTC | #1
On 18/03/2021 16:14, Derek Buitenhuis wrote:
> When extended atom size support was added to probing in
> fec4a2d232d7ebf6d1084fb568d4d84844f25abc, the buffer
> size check was backwards, but probing continued to work
> because there was no minimum size check yet, so despite
> size being 1 on these atoms, and failing to read the 64-bit
> size, the tag was still correctly read.
> 
> When 0b78016b2d7c36b32d07669c0c86bc4b4225ec98 introduced a
> minimum size check, this exposed the bug, and broke probing
> any files with extended atom sizes, such as entirely valid
> large files that start whith mdat atoms.
> 
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
>  libavformat/mov.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I synced the FATE sample and will push these a little later today unless there are comments.

- Derek
diff mbox series

Patch

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 97857789f4..33cfb42228 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -7114,7 +7114,7 @@  static int mov_probe(const AVProbeData *p)
         if ((offset + 8) > (unsigned int)p->buf_size)
             break;
         size = AV_RB32(p->buf + offset);
-        if (size == 1 && offset + 16 > (unsigned int)p->buf_size) {
+        if (size == 1 && offset + 16 <= (unsigned int)p->buf_size) {
             size = AV_RB64(p->buf+offset + 8);
             minsize = 16;
         } else if (size == 0) {