| Message ID | 20240509140211.1296-1-kasper93@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/data_uri: Fix base64 decode buffer size calculation | expand |
On Thu, May 09, 2024 at 04:02:09PM +0200, Kacper Michajłow wrote: > Also reject input if it is too short. > > Found by OSS-Fuzz. > > Signed-off-by: Kacper Michajłow <kasper93@gmail.com> > --- > libavformat/data_uri.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavformat/data_uri.c b/libavformat/data_uri.c > index 3868a19630..f97ecbab37 100644 > --- a/libavformat/data_uri.c > +++ b/libavformat/data_uri.c > @@ -73,11 +73,11 @@ static av_cold int data_open(URLContext *h, const char *uri, int flags) > data++; > in_size = strlen(data); > if (base64) { > - size_t out_size = 3 * (in_size / 4) + 1; > + size_t out_size = AV_BASE64_DECODE_SIZE(in_size); i suspect this is correct > > if (out_size > INT_MAX || !(ddata = av_malloc(out_size))) > return AVERROR(ENOMEM); > - if ((ret = av_base64_decode(ddata, data, out_size)) < 0) { > + if (!out_size || (ret = av_base64_decode(ddata, data, out_size)) < 0) { > av_free(ddata); > av_log(h, AV_LOG_ERROR, "Invalid base64 in URI\n"); > return ret; why would this need a out_size == 0 check ? also it seems av_base64_decode() itself is buggy, ive sent 2 patches fixing av_base64_decode() and extening the self tests thx [...]
On Sat, 11 May 2024 at 03:45, Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Thu, May 09, 2024 at 04:02:09PM +0200, Kacper Michajłow wrote: > > Also reject input if it is too short. > > > > Found by OSS-Fuzz. > > > > Signed-off-by: Kacper Michajłow <kasper93@gmail.com> > > --- > > libavformat/data_uri.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/libavformat/data_uri.c b/libavformat/data_uri.c > > index 3868a19630..f97ecbab37 100644 > > --- a/libavformat/data_uri.c > > +++ b/libavformat/data_uri.c > > @@ -73,11 +73,11 @@ static av_cold int data_open(URLContext *h, const char *uri, int flags) > > data++; > > in_size = strlen(data); > > if (base64) { > > - size_t out_size = 3 * (in_size / 4) + 1; > > + size_t out_size = AV_BASE64_DECODE_SIZE(in_size); > > i suspect this is correct > > > > > > if (out_size > INT_MAX || !(ddata = av_malloc(out_size))) > > return AVERROR(ENOMEM); > > > - if ((ret = av_base64_decode(ddata, data, out_size)) < 0) { > > + if (!out_size || (ret = av_base64_decode(ddata, data, out_size)) < 0) { > > av_free(ddata); > > av_log(h, AV_LOG_ERROR, "Invalid base64 in URI\n"); > > return ret; > > why would this need a out_size == 0 check ? > > also it seems av_base64_decode() itself is buggy, ive sent 2 patches > fixing av_base64_decode() and extening the self tests Thank you for the fix. I can confirm it works. I've still sent an AV_BASE64_DECODE_SIZE change, just for the correctness of it. - Kacper
diff --git a/libavformat/data_uri.c b/libavformat/data_uri.c index 3868a19630..f97ecbab37 100644 --- a/libavformat/data_uri.c +++ b/libavformat/data_uri.c @@ -73,11 +73,11 @@ static av_cold int data_open(URLContext *h, const char *uri, int flags) data++; in_size = strlen(data); if (base64) { - size_t out_size = 3 * (in_size / 4) + 1; + size_t out_size = AV_BASE64_DECODE_SIZE(in_size); if (out_size > INT_MAX || !(ddata = av_malloc(out_size))) return AVERROR(ENOMEM); - if ((ret = av_base64_decode(ddata, data, out_size)) < 0) { + if (!out_size || (ret = av_base64_decode(ddata, data, out_size)) < 0) { av_free(ddata); av_log(h, AV_LOG_ERROR, "Invalid base64 in URI\n"); return ret;
Also reject input if it is too short. Found by OSS-Fuzz. Signed-off-by: Kacper Michajłow <kasper93@gmail.com> --- libavformat/data_uri.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)