Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/hcadec: support decoding with extradata provided in first packet
On 10/4/2023 2:22 PM, Michael Niedermayer wrote: On Mon, Oct 02, 2023 at 10:56:20PM -0300, James Almer wrote: On 10/2/2023 7:23 PM, Michael Niedermayer wrote: Hi On Tue, Sep 05, 2023 at 09:25:45PM +, Paul B Mahol wrote: ffmpeg | branch: master | Paul B Mahol | Tue Sep 5 23:14:58 2023 +0200| [d464a687c9dd03246795d62151809167e8381932] | committer: Paul B Mahol avcodec/hcadec: support decoding with extradata provided in first packet I cannot find this patch on the mailing list Also this adds null pointer writes The init_hca() function which previously was only called once and failure ended all further processing now is called optionally per frame and its failure does not stop further processing so half initialized contexts can be created by an attacker Note, this sort of stuff delays the release thx Does the following fix it? the testcase no longer crashes with this thx Will apply it. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/hcadec: support decoding with extradata provided in first packet
On Mon, Oct 02, 2023 at 10:56:20PM -0300, James Almer wrote: > On 10/2/2023 7:23 PM, Michael Niedermayer wrote: > > Hi > > > > On Tue, Sep 05, 2023 at 09:25:45PM +, Paul B Mahol wrote: > > > ffmpeg | branch: master | Paul B Mahol | Tue Sep 5 > > > 23:14:58 2023 +0200| [d464a687c9dd03246795d62151809167e8381932] | > > > committer: Paul B Mahol > > > > > > avcodec/hcadec: support decoding with extradata provided in first packet > > > > I cannot find this patch on the mailing list > > > > Also this adds null pointer writes > > The init_hca() function which previously was only called once and failure > > ended all further processing now is called optionally per frame and its > > failure does not stop further processing so half initialized contexts > > can be created by an attacker > > > > Note, this sort of stuff delays the release > > > > thx > > Does the following fix it? the testcase no longer crashes with this thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The worst form of inequality is to try to make unequal things equal. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/hcadec: support decoding with extradata provided in first packet
On 10/2/2023 7:23 PM, Michael Niedermayer wrote:
Hi
On Tue, Sep 05, 2023 at 09:25:45PM +, Paul B Mahol wrote:
ffmpeg | branch: master | Paul B Mahol | Tue Sep 5 23:14:58
2023 +0200| [d464a687c9dd03246795d62151809167e8381932] | committer: Paul B Mahol
avcodec/hcadec: support decoding with extradata provided in first packet
I cannot find this patch on the mailing list
Also this adds null pointer writes
The init_hca() function which previously was only called once and failure
ended all further processing now is called optionally per frame and its
failure does not stop further processing so half initialized contexts
can be created by an attacker
Note, this sort of stuff delays the release
thx
Does the following fix it?
diff --git a/libavcodec/hcadec.c b/libavcodec/hcadec.c
index 6f277afb96..4e30d553de 100644
--- a/libavcodec/hcadec.c
+++ b/libavcodec/hcadec.c
@@ -65,6 +65,7 @@ typedef struct HCAContext {
uint8_t stereo_band_count;
uint8_t bands_per_hfr_group;
+// Set during init() and freed on close(). Untouched on flush()
av_tx_fn tx_fn;
AVTXContext *tx_ctx;
AVFloatDSPContext *fdsp;
@@ -196,6 +197,13 @@ static inline unsigned ceil2(unsigned a, unsigned b)
return (b > 0) ? (a / b + ((a % b) ? 1 : 0)) : 0;
}
+static av_cold void decode_flush(AVCodecContext *avctx)
+{
+HCAContext *c = avctx->priv_data;
+
+memset(c, 0, offsetof(HCAContext, tx_fn));
+}
+
static int init_hca(AVCodecContext *avctx, const uint8_t *extradata,
const int extradata_size)
{
@@ -205,6 +213,8 @@ static int init_hca(AVCodecContext *avctx, const uint8_t
*extradata,
unsigned b, chunk;
int version, ret;
+decode_flush(avctx);
+
if (extradata_size < 36)
return AVERROR_INVALIDDATA;
@@ -340,6 +350,9 @@ static int init_hca(AVCodecContext *avctx, const uint8_t
*extradata,
return AVERROR_INVALIDDATA;
}
+// Done last to signal init() finished
+c->crc_table = av_crc_get_table(AV_CRC_16_ANSI);
+
return 0;
}
@@ -350,7 +363,6 @@ static av_cold int decode_init(AVCodecContext *avctx)
int ret;
avctx->sample_fmt = AV_SAMPLE_FMT_FLTP;
-c->crc_table = av_crc_get_table(AV_CRC_16_ANSI);
if (avctx->ch_layout.nb_channels <= 0 || avctx->ch_layout.nb_channels >
FF_ARRAY_ELEMS(c->ch))
return AVERROR(EINVAL);
@@ -534,6 +546,9 @@ static int decode_frame(AVCodecContext *avctx, AVFrame
*frame,
}
}
+if (!c->crc_table)
+return AVERROR_INVALIDDATA;
+
if (c->key || c->subkey) {
uint8_t *data, *cipher = c->cipher;
@@ -602,6 +617,7 @@ const FFCodec ff_hca_decoder = {
.priv_data_size = sizeof(HCAContext),
.init = decode_init,
FF_CODEC_DECODE_CB(decode_frame),
+.flush = decode_flush,
.close = decode_close,
.p.capabilities = AV_CODEC_CAP_DR1,
.caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/hcadec: support decoding with extradata provided in first packet
Hi
On Tue, Sep 05, 2023 at 09:25:45PM +, Paul B Mahol wrote:
> ffmpeg | branch: master | Paul B Mahol | Tue Sep 5
> 23:14:58 2023 +0200| [d464a687c9dd03246795d62151809167e8381932] | committer:
> Paul B Mahol
>
> avcodec/hcadec: support decoding with extradata provided in first packet
I cannot find this patch on the mailing list
Also this adds null pointer writes
The init_hca() function which previously was only called once and failure
ended all further processing now is called optionally per frame and its
failure does not stop further processing so half initialized contexts
can be created by an attacker
Note, this sort of stuff delays the release
thx
[...]
> @@ -385,16 +404,34 @@ static int decode_frame(AVCodecContext *avctx, AVFrame
> *frame,
> int *got_frame_ptr, AVPacket *avpkt)
> {
> HCAContext *c = avctx->priv_data;
> -int ch, ret, packed_noise_level;
> +int ch, offset = 0, ret, packed_noise_level;
> GetBitContext gb0, *const gb =
> float **samples;
>
> +if (avpkt->size <= 8)
> +return AVERROR_INVALIDDATA;
> +
> +if (AV_RN16(avpkt->data) != 0x) {
> +if (AV_RL32(avpkt->data) != MKTAG('H','C','A',0)) {
> +return AVERROR_INVALIDDATA;
> +} else if (AV_RB16(avpkt->data + 6) <= avpkt->size) {
> +ret = init_hca(avctx, avpkt->data, AV_RB16(avpkt->data + 6));
> +if (ret < 0)
> +return ret;
> +offset = AV_RB16(avpkt->data + 6);
> +if (offset == avpkt->size)
> +return avpkt->size;
> +} else {
> +return AVERROR_INVALIDDATA;
> +}
> +}
[...]
==6921==ERROR: AddressSanitizer: SEGV on unknown address 0x (pc
0x005c8da1 bp 0x7fff5a918390 sp 0x7fff5a918200 T0)
==6921==The signal is caused by a WRITE memory access.
==6921==Hint: address points to the zero page.
#0 0x5c8da1 in unpack /src/ffmpeg/libavcodec/hcadec.c:392:30
#1 0x5c8da1 in decode_frame /src/ffmpeg/libavcodec/hcadec.c:456:9
#2 0x5b41ff in decode_simple_internal /src/ffmpeg/libavcodec/decode.c:433:20
#3 0x5b41ff in decode_simple_receive_frame
/src/ffmpeg/libavcodec/decode.c:607:15
#4 0x5b41ff in decode_receive_frame_internal
/src/ffmpeg/libavcodec/decode.c:635:15
#5 0x5b3aee in avcodec_send_packet /src/ffmpeg/libavcodec/decode.c:732:15
#6 0x59f64f in LLVMFuzzerTestOneInput
/src/ffmpeg/tools/target_dec_fuzzer.c:522:25
#7 0x46e913 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*,
unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
#8 0x45a072 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned
long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
#9 0x45f91c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char
const*, unsigned long))
/src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
#10 0x488e52 in main
/src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#11 0x7849ef29a082 in __libc_start_main
/build/glibc-SzIz7B/glibc-2.31/csu/libc-start.c:308:16
#12 0x45023d in _start
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
than the original author, trying to rewrite it will not make it better.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
