From patchwork Mon Apr 27 15:19:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mattias Wadman X-Patchwork-Id: 19295 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 731A24492CE for ; Mon, 27 Apr 2020 19:14:54 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5172868BB98; Mon, 27 Apr 2020 19:14:54 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-ed1-f66.google.com (mail-ed1-f66.google.com [209.85.208.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 80CFC68BAD2 for ; Mon, 27 Apr 2020 19:14:48 +0300 (EEST) Received: by mail-ed1-f66.google.com with SMTP id g16so13909414eds.1 for ; Mon, 27 Apr 2020 09:14:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=xjVeoZHru6IQ438b2a8RmnBwNZp/Ll8Yn9IRDRqSGIc=; b=U+Qm8YVQQss+BlQ+Fp8M0bRPBpJO9E8a4+m6BO9xVfobnocj/o9Z6C3Unnqxjx1MPa VCoqe+ZvAFexsiXdn4wtFvbCYQSPTBFQfjycRUjfj4uQFf+0x84z0q/0FGLelvPCl9yD 5wJg8wT4XHSZwHTdGp+y/kmqMwE9qwEnAhuh0Fv2/6OQCb0awgogWkJC0hRlpIPDUg4+ Qqy/nkdVZvV/mu0KeSUrCv2T3gphxlxnmHjqXY4jEpSwEiJJP1z/x9VDgwSd8jTA0xDR kYuEOf4cgqucishCz910mUSKqq3fLQVt9MskDTfcbodQds35QfeeFaKdgQqesqJeyWYy AQJQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=xjVeoZHru6IQ438b2a8RmnBwNZp/Ll8Yn9IRDRqSGIc=; b=rKsnXyaOLLqASklxUU+w+67nOtoJsAyJVPtgljIqThd/bjJAkM0f+Ah1DkFXryWWjq tHDozOwfsTPF2MVC16jjCAG2cEpBYZkuIoZgbddiBOcXM+J9F5cABYhMvhRJyAbuBJ4w TRGQ5DIteQS7nq2uXwMf/94Y3f8jYdnTMDN+d+b3Uea7ThsZo6Q5xLE+4e2BWWp76hYK nhE8+dNKlf5ItwvGGd4MY107x6LP2FVKf1+uy9Vc7o6iMqqK88aDG8R7cRKSWz1AWxyf VM6tTGPUFH7weB1GF/GJSFawx5NJqhOqdgqhvNS0MKhuLWgsbHuiqu42mt3fZKMumtFF AQEA== X-Gm-Message-State: AGi0Puapq9eVN6ryiFAu9jl2WnrooCopyLi9F4M9omlTdMNg0c4Jfbgt GaSLBhEh0XPS+qxL+3J1UJb/zmx+qWw= X-Google-Smtp-Source: APiQypIQPgkcZczmdrb3onw9FVL/H+m4I2YUsu1dgR7P+u0AceGFS1paCKXIrP7XxazJmwOTskWm0g== X-Received: by 2002:ac2:5b92:: with SMTP id o18mr12096611lfn.140.1588000880496; Mon, 27 Apr 2020 08:21:20 -0700 (PDT) Received: from localhost.localdomain (host-95-192-155-129.mobileonline.telia.com. [95.192.155.129]) by smtp.gmail.com with ESMTPSA id y199sm11714512lfa.80.2020.04.27.08.21.18 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 27 Apr 2020 08:21:18 -0700 (PDT) From: Mattias Wadman To: ffmpeg-devel@ffmpeg.org Date: Mon, 27 Apr 2020 17:19:49 +0200 Message-Id: <20200427151948.91897-1-mattias.wadman@gmail.com> X-Mailer: git-send-email 2.18.0 Subject: [FFmpeg-devel] [PATCH] avformat/oggdec: Add page CRC verification 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 Cc: Mattias Wadman MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Fixes seek issue with ogg files that have segment data that happens to be encoded as a "OggS" page syncword. Very unlikely but seems to happen. Have been observed to happen with ffmpeg+libvorbis and oggenc encoding to 441khz stereo at 160kbps. --- libavformat/oggdec.c | 96 +++++++++++++++++++++++++++++++++----------- 1 file changed, 73 insertions(+), 23 deletions(-) diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c index 95190589ab..22532f0341 100644 --- a/libavformat/oggdec.c +++ b/libavformat/oggdec.c @@ -31,6 +31,8 @@ #include #include "libavutil/avassert.h" #include "libavutil/intreadwrite.h" +#include "libavutil/crc.h" +#include "libavcodec/bytestream.h" #include "oggdec.h" #include "avformat.h" #include "internal.h" @@ -339,14 +341,24 @@ static int ogg_read_page(AVFormatContext *s, int *sid) AVIOContext *bc = s->pb; struct ogg *ogg = s->priv_data; struct ogg_stream *os; - int ret, i = 0; - int flags, nsegs; + int ret, i; + int version, flags, nsegs; uint64_t gp; uint32_t serial; + uint32_t crc; int size, idx; uint8_t sync[4]; - int sp = 0; - + uint8_t header[23]; + GetByteContext headergb; + uint8_t segments[255]; + uint8_t *segmentsdata; + int sp; + const AVCRC *crc_table; + uint32_t ccrc; + +again: + sp = 0; + i = 0; ret = avio_read(bc, sync, 4); if (ret < 4) return ret < 0 ? ret : AVERROR_EOF; @@ -378,19 +390,59 @@ static int ogg_read_page(AVFormatContext *s, int *sid) return AVERROR_INVALIDDATA; } - if (avio_r8(bc) != 0) { /* version */ - av_log (s, AV_LOG_ERROR, "ogg page, unsupported version\n"); + ret = avio_read(bc, header, sizeof(header)); + if (ret < sizeof(header)) + return AVERROR_INVALIDDATA; + nsegs = header[22]; + ret = avio_read(bc, segments, nsegs); + if (ret < nsegs) + return AVERROR_INVALIDDATA; + size = 0; + for (i = 0; i < nsegs; i++) + size += segments[i]; + + bytestream2_init(&headergb, header, sizeof(header)); + version = bytestream2_get_byte(&headergb); + flags = bytestream2_get_byte(&headergb); + gp = bytestream2_get_le64(&headergb); + serial = bytestream2_get_le32(&headergb); + bytestream2_skip(&headergb, 4); // seq le32 + crc = bytestream2_get_le32(&headergb); + + segmentsdata = av_malloc(size); + if (!segmentsdata) + return AVERROR(ENOMEM); + ret = avio_read(bc, segmentsdata, size); + if (ret < size) { + av_freep(&segmentsdata); return AVERROR_INVALIDDATA; } - flags = avio_r8(bc); - gp = avio_rl64(bc); - serial = avio_rl32(bc); - avio_skip(bc, 8); /* seq, crc */ - nsegs = avio_r8(bc); + // Reset CRC in header to zero and calculate for whole page + crc_table = av_crc_get_table(AV_CRC_32_IEEE); + memset(&header[18], 0, 4); + ccrc = 0; + ccrc = av_crc(crc_table, ccrc, "OggS", 4); + ccrc = av_crc(crc_table, ccrc, header, sizeof(header)); + ccrc = av_crc(crc_table, ccrc, segments, nsegs); + ccrc = av_crc(crc_table, ccrc, segmentsdata, size); + // Default AV_CRC_32_IEEE table is BE + ccrc = av_bswap32(ccrc); + + if (ccrc != crc) { + av_log(s, AV_LOG_TRACE, "ogg page, invalid checksum %x != %x\n", ccrc, crc); + if (s->error_recognition & AV_EF_CRCCHECK) { + av_freep(&segmentsdata); + // TODO: smarter use of read data? + goto again; + } + } - if (avio_feof(bc)) - return AVERROR_EOF; + if (version != 0) { + av_log(s, AV_LOG_ERROR, "ogg page, unsupported version %d\n", version); + av_freep(&segmentsdata); + return AVERROR_INVALIDDATA; + } idx = ogg_find_stream(ogg, serial); if (idx < 0) { @@ -401,24 +453,24 @@ static int ogg_read_page(AVFormatContext *s, int *sid) if (idx < 0) { av_log(s, AV_LOG_ERROR, "failed to create or replace stream\n"); + av_freep(&segmentsdata); return idx; } } os = ogg->streams + idx; ogg->page_pos = - os->page_pos = avio_tell(bc) - 27; + os->page_pos = avio_tell(bc) - 27 - size - nsegs;; if (os->psize > 0) { ret = ogg_new_buf(ogg, idx); - if (ret < 0) + if (ret < 0) { + av_freep(&segmentsdata); return ret; + } } - ret = avio_read(bc, os->segments, nsegs); - if (ret < nsegs) - return ret < 0 ? ret : AVERROR_EOF; - + memcpy(os->segments, segments, nsegs); os->nsegs = nsegs; os->segp = 0; @@ -456,10 +508,8 @@ static int ogg_read_page(AVFormatContext *s, int *sid) os->buf = nb; } - ret = avio_read(bc, os->buf + os->bufpos, size); - if (ret < size) - return ret < 0 ? ret : AVERROR_EOF; - + memcpy(os->buf + os->bufpos, segmentsdata, size); + av_freep(&segmentsdata); os->bufpos += size; os->granule = gp; os->flags = flags;