From patchwork Fri Sep 6 11:11:50 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pascal Massimino X-Patchwork-Id: 14955 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 3992544A637 for ; Fri, 6 Sep 2019 14:19:17 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 01744687FF8; Fri, 6 Sep 2019 14:19:17 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-lf1-f65.google.com (mail-lf1-f65.google.com [209.85.167.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6CA85687FF3 for ; Fri, 6 Sep 2019 14:19:11 +0300 (EEST) Received: by mail-lf1-f65.google.com with SMTP id u13so4682508lfm.9 for ; Fri, 06 Sep 2019 04:19:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=/p48G6crcr7AVJe0P9mw4bZ4J363cqkj73Eg6MlMWRw=; b=tg/4OiOIYgMj8+rsYgzWr0u9AcQI2NjKaXRqibok00/KEP6WKnfKGkBMArjv1NoJA0 oJZiPKe+GTCA4Libm46VYCTdPva/geV8Q1JoNPkKXaoHz35zkirEIQWx9JxijpjenoEk GXsMxzlWNT9/W68NFa/aq965d4xlAzo9FrnTiiTZY6iJqaxrBKnRArg8bu6DuLpr/aRR Ne9p3rPH/DHWVCCorv0YQnFs0LVhBwKXP8akRjZuB/M6c3BbDiLgVY78gFCAZYHJc7a5 J7mzWpaQBdFflKFjwRyEA/2NFxuOpeDGqWa1+X5Dh8sGZuActnGoHPf0/CN0gpDdc6xI t61g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=/p48G6crcr7AVJe0P9mw4bZ4J363cqkj73Eg6MlMWRw=; b=i87OIvyCg8jzOlZDPLcTvGhpgUg3YBCFah1iqc41xu1v22Xpy2oqd5ghBtMVDYt5Ho PxC+548XZQeVRkEG1KkqwtEjOaFaJwu7VGaAc4uKliBM7H6K+6iQ3X04SH22j9wna/kX P41VsezG/0NvRiSyK0TRTMX1n4D4p6FC2NsI+cfODLnOLcwphA+I6DMfN7z/LE1qDLDz q25CfJBwTDVlDnjeGfA3byMe5t0YzMFDvdVMVmXxhvTlXfRmxB3fgxm4ToKRGHMwk1Cw xfwA5RnphQ36g5XeEuh2GnfpBaZBs/Cf4qPZgRTFwTGX3QO5TeYW9ydQqDK2rHbi3geV gzQQ== X-Gm-Message-State: APjAAAWzDXfxE9le9vWxIIuKahS03KLTflCRkZ2lP3qqMdfho1T7KMzs T894jy44g7ojR8CanyPhBYHyBfDM3r56DHYjjU89Fw== X-Google-Smtp-Source: APXvYqxtPqPhYa7YjMZcafJtqu7U2dJlqClHo/9MtmuDQzTcQtkHhH8uTU3mZlnmFRUsBCLICGUEi+pJruqlemqqCCE= X-Received: by 2002:a19:3f47:: with SMTP id m68mr5817562lfa.108.1567768321682; Fri, 06 Sep 2019 04:12:01 -0700 (PDT) MIME-Version: 1.0 References: <20190905162024.GY3219@michaelspb> In-Reply-To: <20190905162024.GY3219@michaelspb> From: Pascal Massimino Date: Fri, 6 Sep 2019 13:11:50 +0200 Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.20 Subject: Re: [FFmpeg-devel] [PATCH] avcodec/webp: avoid trying to decode trailing junk in bitstreams X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Michael, On Thu, Sep 5, 2019 at 6:20 PM Michael Niedermayer wrote: > On Wed, Sep 04, 2019 at 07:43:15AM +0200, Pascal Massimino wrote: > > Hi, > > > > this patch break the decoding loop when invalid webp chunk is > encountered. > > We can still have a valid frame ready to be returned (*got_frame = 1). > > > > > fixes trac #8107 (/#7612) > > These bug references should be in the commit message > done, new patch attached. > > > > > > skal/ > > > webp.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > 2d80b062adade6044f64a00838b55f9427cc1f73 > 0001-webp-fix-decoding-for-trailing-junk.patch > > From 9edff4f9812fad7f605bdc12954f82a8745a25ee Mon Sep 17 00:00:00 2001 > > From: Pascal Massimino > > Date: Wed, 28 Aug 2019 09:41:42 +0200 > > Subject: [PATCH] webp: fix decoding for trailing junk > > > > some bitstream have trailing junk, despite being valid webp data. > > In case of apparent error, abort the loop and let *got_frame > > decide whether this is an error or not. > > Another possibility would be turning the loop into: > > while (!*got_frame) {...} > > what is that trailing junk ? > As mentioned in the trac description, one frequent source of such junk is padding of encrypted packets (or, just, left-overs). > > i would guess its not a known chunk but rather hits the default > is that just a bunch of 0 or 0xFF bytes ? > detecting before we read into the end feels more robust if > we can simply detect the "junk" > As i mentioned in the description, a possibly more robust solution would be just stopping the loop as soon as 'chunk_size' bytes have been consumed (leading to *got_frame = 1) and no more. This current patch is minimalist, though. skal/ > > thanks > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". From 5c3a8878cce7e2920c39e58a0b57872d8c96b511 Mon Sep 17 00:00:00 2001 From: Pascal Massimino Date: Wed, 28 Aug 2019 09:41:42 +0200 Subject: [PATCH] avcodec/webp: fix decoding for trailing junk some bitstream have trailing junk, despite being valid webp data. In case of apparent error, abort the loop and let *got_frame decide whether this is an error or not. fixes trac #8107 (/#7612) Another possibility would be turning the loop into: while (!*got_frame) {...} --- libavcodec/webp.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libavcodec/webp.c b/libavcodec/webp.c index 077bb06f85..c6d0206846 100644 --- a/libavcodec/webp.c +++ b/libavcodec/webp.c @@ -1412,8 +1412,11 @@ static int webp_decode_frame(AVCodecContext *avctx, void *data, int *got_frame, return AVERROR_INVALIDDATA; chunk_size += chunk_size & 1; - if (bytestream2_get_bytes_left(&gb) < chunk_size) - return AVERROR_INVALIDDATA; + if (bytestream2_get_bytes_left(&gb) < chunk_size) { + /* we seem to be running out of data, but it could also be that the + bitstream has trailing junk leading to bogus chunk_size. */ + break; + } switch (chunk_type) { case MKTAG('V', 'P', '8', ' '): -- 2.23.0.187.g17f5b7556c-goog