[FFmpeg-devel] [PATCH] avformat/data_uri: Fix base64 decode buffer size calculation

[FFmpeg-devel] [PATCH] avformat/data_uri: Fix base64 decode buffer size calculation

[Kacper Michajłow]

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);
 
         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;
-- 
2.43.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat/data_uri: Fix base64 decode buffer size calculation

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1556 bytes --]

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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat/data_uri: Fix base64 decode buffer size calculation

[Kacper Michajlow]

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
_______________________________________________
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".