| Message ID | 20200629190013.26657-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | b5e39880fb7269b1b3577cee288e06aa3dc1dfa2 |
| Headers | show |
| Series | [FFmpeg-devel] avformat/hls: Pass a copy of the URL for probing | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Michael Niedermayer: > The segments / url can be modified by the io read when reloading > > This may be an alternative or additional fix for Ticket8673 > as a further alternative the reload stuff could be disabled during > probing > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/hls.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/libavformat/hls.c b/libavformat/hls.c > index 3798bdd5d1..ba17c4ed96 100644 > --- a/libavformat/hls.c > +++ b/libavformat/hls.c > @@ -1946,6 +1946,7 @@ static int hls_read_header(AVFormatContext *s) > /* Open the demuxer for each playlist */ > for (i = 0; i < c->n_playlists; i++) { > struct playlist *pls = c->playlists[i]; > + char *url; > ff_const59 AVInputFormat *in_fmt = NULL; > > if (!(pls->ctx = avformat_alloc_context())) { > @@ -1983,8 +1984,9 @@ static int hls_read_header(AVFormatContext *s) > read_data, NULL, NULL); > pls->ctx->probesize = s->probesize > 0 ? s->probesize : 1024 * 4; > pls->ctx->max_analyze_duration = s->max_analyze_duration > 0 ? s->max_analyze_duration : 4 * AV_TIME_BASE; > - ret = av_probe_input_buffer(&pls->pb, &in_fmt, pls->segments[0]->url, > - NULL, 0, 0); > + url = av_strdup(pls->segments[0]->url); > + ret = av_probe_input_buffer(&pls->pb, &in_fmt, url, NULL, 0, 0); > + av_free(url); > if (ret < 0) { > /* Free the ctx - it isn't initialized properly at this point, > * so avformat_close_input shouldn't be called. If > Is the missing allocation check intentional? - Andreas
On Mon, Jun 29, 2020 at 09:08:07PM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > The segments / url can be modified by the io read when reloading > > > > This may be an alternative or additional fix for Ticket8673 > > as a further alternative the reload stuff could be disabled during > > probing > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/hls.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/libavformat/hls.c b/libavformat/hls.c > > index 3798bdd5d1..ba17c4ed96 100644 > > --- a/libavformat/hls.c > > +++ b/libavformat/hls.c > > @@ -1946,6 +1946,7 @@ static int hls_read_header(AVFormatContext *s) > > /* Open the demuxer for each playlist */ > > for (i = 0; i < c->n_playlists; i++) { > > struct playlist *pls = c->playlists[i]; > > + char *url; > > ff_const59 AVInputFormat *in_fmt = NULL; > > > > if (!(pls->ctx = avformat_alloc_context())) { > > @@ -1983,8 +1984,9 @@ static int hls_read_header(AVFormatContext *s) > > read_data, NULL, NULL); > > pls->ctx->probesize = s->probesize > 0 ? s->probesize : 1024 * 4; > > pls->ctx->max_analyze_duration = s->max_analyze_duration > 0 ? s->max_analyze_duration : 4 * AV_TIME_BASE; > > - ret = av_probe_input_buffer(&pls->pb, &in_fmt, pls->segments[0]->url, > > - NULL, 0, 0); > > + url = av_strdup(pls->segments[0]->url); > > + ret = av_probe_input_buffer(&pls->pb, &in_fmt, url, NULL, 0, 0); > > + av_free(url); > > if (ret < 0) { > > /* Free the ctx - it isn't initialized properly at this point, > > * so avformat_close_input shouldn't be called. If > > > Is the missing allocation check intentional? i thought if it fails filename is NULL and the code could still plausibly continue. but we can also fail or warn if people prefer thx [...]
On Mon, Jun 29, 2020 at 09:18:18PM +0200, Michael Niedermayer wrote: > On Mon, Jun 29, 2020 at 09:08:07PM +0200, Andreas Rheinhardt wrote: > > Michael Niedermayer: > > > The segments / url can be modified by the io read when reloading > > > > > > This may be an alternative or additional fix for Ticket8673 > > > as a further alternative the reload stuff could be disabled during > > > probing > > > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > --- > > > libavformat/hls.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/libavformat/hls.c b/libavformat/hls.c > > > index 3798bdd5d1..ba17c4ed96 100644 > > > --- a/libavformat/hls.c > > > +++ b/libavformat/hls.c > > > @@ -1946,6 +1946,7 @@ static int hls_read_header(AVFormatContext *s) > > > /* Open the demuxer for each playlist */ > > > for (i = 0; i < c->n_playlists; i++) { > > > struct playlist *pls = c->playlists[i]; > > > + char *url; > > > ff_const59 AVInputFormat *in_fmt = NULL; > > > > > > if (!(pls->ctx = avformat_alloc_context())) { > > > @@ -1983,8 +1984,9 @@ static int hls_read_header(AVFormatContext *s) > > > read_data, NULL, NULL); > > > pls->ctx->probesize = s->probesize > 0 ? s->probesize : 1024 * 4; > > > pls->ctx->max_analyze_duration = s->max_analyze_duration > 0 ? s->max_analyze_duration : 4 * AV_TIME_BASE; > > > - ret = av_probe_input_buffer(&pls->pb, &in_fmt, pls->segments[0]->url, > > > - NULL, 0, 0); > > > + url = av_strdup(pls->segments[0]->url); > > > + ret = av_probe_input_buffer(&pls->pb, &in_fmt, url, NULL, 0, 0); > > > + av_free(url); > > > if (ret < 0) { > > > /* Free the ctx - it isn't initialized properly at this point, > > > * so avformat_close_input shouldn't be called. If > > > > > Is the missing allocation check intentional? > > i thought if it fails filename is NULL and the code could still > plausibly continue. > but we can also fail or warn if people prefer will apply this as i think this makes the fix for a public CVE more robust [...]
diff --git a/libavformat/hls.c b/libavformat/hls.c index 3798bdd5d1..ba17c4ed96 100644 --- a/libavformat/hls.c +++ b/libavformat/hls.c @@ -1946,6 +1946,7 @@ static int hls_read_header(AVFormatContext *s) /* Open the demuxer for each playlist */ for (i = 0; i < c->n_playlists; i++) { struct playlist *pls = c->playlists[i]; + char *url; ff_const59 AVInputFormat *in_fmt = NULL; if (!(pls->ctx = avformat_alloc_context())) { @@ -1983,8 +1984,9 @@ static int hls_read_header(AVFormatContext *s) read_data, NULL, NULL); pls->ctx->probesize = s->probesize > 0 ? s->probesize : 1024 * 4; pls->ctx->max_analyze_duration = s->max_analyze_duration > 0 ? s->max_analyze_duration : 4 * AV_TIME_BASE; - ret = av_probe_input_buffer(&pls->pb, &in_fmt, pls->segments[0]->url, - NULL, 0, 0); + url = av_strdup(pls->segments[0]->url); + ret = av_probe_input_buffer(&pls->pb, &in_fmt, url, NULL, 0, 0); + av_free(url); if (ret < 0) { /* Free the ctx - it isn't initialized properly at this point, * so avformat_close_input shouldn't be called. If
The segments / url can be modified by the io read when reloading This may be an alternative or additional fix for Ticket8673 as a further alternative the reload stuff could be disabled during probing Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/hls.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)