[FFmpeg-devel] [PATCH] lavu/avstring: check for overlong encodings
Fix reopened trac ticket #1163.
---
libavutil/avstring.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/libavutil/avstring.c b/libavutil/avstring.c
index a63fb84..df27d5e 100644
--- a/libavutil/avstring.c
+++ b/libavutil/avstring.c
@@ -331,7 +331,10 @@ int av_utf8_decode(int32_t *codep, const uint8_t **bufp,
const uint8_t *buf_end,
const uint8_t *p = *bufp;
uint32_t top;
uint64_t code;
-int ret = 0;
+int ret = 0, tail_len;
+uint32_t overlong_encoding_mins[6] = {
+0x, 0x0080, 0x0800, 0x0001, 0x0020, 0x0400,
+};
if (p >= buf_end)
return 0;
@@ -346,8 +349,10 @@ int av_utf8_decode(int32_t *codep, const uint8_t **bufp,
const uint8_t *buf_end,
}
top = (code & 128) >> 1;
+tail_len = 0;
while (code & top) {
int tmp;
+tail_len++;
if (p >= buf_end) {
(*bufp) ++;
return AVERROR(EILSEQ); /* incomplete sequence */
@@ -364,6 +369,12 @@ int av_utf8_decode(int32_t *codep, const uint8_t **bufp,
const uint8_t *buf_end,
}
code &= (top << 1) - 1;
+/* check for overlong encodings */
+if (code < overlong_encoding_mins[tail_len]) {
+ret = AVERROR(EILSEQ);
+goto end;
+}
+
if (code >= 1<<31) {
ret = AVERROR(EILSEQ); /* out-of-range value */
goto end;
--
1.8.3.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavu/avstring: check for overlong encodings
On Thu, 28 Aug 2014 17:39:27 +0200
Stefano Sabatini wrote:
> Fix reopened trac ticket #1163.
> ---
> libavutil/avstring.c | 13 -
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/libavutil/avstring.c b/libavutil/avstring.c
> index a63fb84..df27d5e 100644
> --- a/libavutil/avstring.c
> +++ b/libavutil/avstring.c
> @@ -331,7 +331,10 @@ int av_utf8_decode(int32_t *codep, const uint8_t **bufp,
> const uint8_t *buf_end,
> const uint8_t *p = *bufp;
> uint32_t top;
> uint64_t code;
> -int ret = 0;
> +int ret = 0, tail_len;
> +uint32_t overlong_encoding_mins[6] = {
> +0x, 0x0080, 0x0800, 0x0001, 0x0020,
> 0x0400,
> +};
>
> if (p >= buf_end)
> return 0;
> @@ -346,8 +349,10 @@ int av_utf8_decode(int32_t *codep, const uint8_t **bufp,
> const uint8_t *buf_end,
> }
> top = (code & 128) >> 1;
>
> +tail_len = 0;
> while (code & top) {
> int tmp;
> +tail_len++;
> if (p >= buf_end) {
> (*bufp) ++;
> return AVERROR(EILSEQ); /* incomplete sequence */
> @@ -364,6 +369,12 @@ int av_utf8_decode(int32_t *codep, const uint8_t **bufp,
> const uint8_t *buf_end,
> }
> code &= (top << 1) - 1;
>
> +/* check for overlong encodings */
> +if (code < overlong_encoding_mins[tail_len]) {
> +ret = AVERROR(EILSEQ);
> +goto end;
> +}
> +
> if (code >= 1<<31) {
> ret = AVERROR(EILSEQ); /* out-of-range value */
> goto end;
Looks ok and simple to me. Is there a guarantee tail_len never
becomes larger than 5?
Also note that libavcodec/utils.c contains the same check (but less
readable) in utf8_check().
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavu/avstring: check for overlong encodings
On date Friday 2014-08-29 15:57:32 +0200, wm4 encoded:
> On Thu, 28 Aug 2014 17:39:27 +0200
> Stefano Sabatini wrote:
>
> > Fix reopened trac ticket #1163.
> > ---
> > libavutil/avstring.c | 13 -
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavutil/avstring.c b/libavutil/avstring.c
> > index a63fb84..df27d5e 100644
> > --- a/libavutil/avstring.c
> > +++ b/libavutil/avstring.c
> > @@ -331,7 +331,10 @@ int av_utf8_decode(int32_t *codep, const uint8_t
> > **bufp, const uint8_t *buf_end,
> > const uint8_t *p = *bufp;
> > uint32_t top;
> > uint64_t code;
> > -int ret = 0;
> > +int ret = 0, tail_len;
> > +uint32_t overlong_encoding_mins[6] = {
> > +0x, 0x0080, 0x0800, 0x0001, 0x0020,
> > 0x0400,
> > +};
> >
> > if (p >= buf_end)
> > return 0;
> > @@ -346,8 +349,10 @@ int av_utf8_decode(int32_t *codep, const uint8_t
> > **bufp, const uint8_t *buf_end,
> > }
> > top = (code & 128) >> 1;
> >
> > +tail_len = 0;
> > while (code & top) {
> > int tmp;
> > +tail_len++;
> > if (p >= buf_end) {
> > (*bufp) ++;
> > return AVERROR(EILSEQ); /* incomplete sequence */
> > @@ -364,6 +369,12 @@ int av_utf8_decode(int32_t *codep, const uint8_t
> > **bufp, const uint8_t *buf_end,
> > }
> > code &= (top << 1) - 1;
> >
> > +/* check for overlong encodings */
> > +if (code < overlong_encoding_mins[tail_len]) {
> > +ret = AVERROR(EILSEQ);
> > +goto end;
> > +}
> > +
> > if (code >= 1<<31) {
> > ret = AVERROR(EILSEQ); /* out-of-range value */
> > goto end;
>
> Looks ok and simple to me. Is there a guarantee tail_len never
> becomes larger than 5?
It is mathematically impossible that the length will be larger than 5.
Added an assert for that though, alternatively I could add a check.
>
> Also note that libavcodec/utils.c contains the same check (but less
> readable) in utf8_check().
--
FFmpeg = Faithful Fundamentalist Most Power Enhancing Goblin
>From 75db7cc2ea2b7b12e611b7b3fd103aee4f707dde Mon Sep 17 00:00:00 2001
From: Stefano Sabatini
Date: Thu, 28 Aug 2014 17:37:27 +0200
Subject: [PATCH] lavu/avstring: check for overlong encodings
Fix reopened trac ticket #1163.
---
libavutil/avstring.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/libavutil/avstring.c b/libavutil/avstring.c
index a63fb84..fd010e4 100644
--- a/libavutil/avstring.c
+++ b/libavutil/avstring.c
@@ -27,6 +27,7 @@
#include "config.h"
#include "common.h"
#include "mem.h"
+#include "avassert.h"
#include "avstring.h"
#include "bprint.h"
@@ -331,7 +332,10 @@ int av_utf8_decode(int32_t *codep, const uint8_t **bufp, const uint8_t *buf_end,
const uint8_t *p = *bufp;
uint32_t top;
uint64_t code;
-int ret = 0;
+int ret = 0, tail_len;
+uint32_t overlong_encoding_mins[6] = {
+0x, 0x0080, 0x0800, 0x0001, 0x0020, 0x0400,
+};
if (p >= buf_end)
return 0;
@@ -346,8 +350,10 @@ int av_utf8_decode(int32_t *codep, const uint8_t **bufp, const uint8_t *buf_end,
}
top = (code & 128) >> 1;
+tail_len = 0;
while (code & top) {
int tmp;
+tail_len++;
if (p >= buf_end) {
(*bufp) ++;
return AVERROR(EILSEQ); /* incomplete sequence */
@@ -364,6 +370,13 @@ int av_utf8_decode(int32_t *codep, const uint8_t **bufp, const uint8_t *buf_end,
}
code &= (top << 1) - 1;
+/* check for overlong encodings */
+av_assert0(tail_len <= 5);
+if (code < overlong_encoding_mins[tail_len]) {
+ret = AVERROR(EILSEQ);
+goto end;
+}
+
if (code >= 1<<31) {
ret = AVERROR(EILSEQ); /* out-of-range value */
goto end;
--
1.8.3.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavu/avstring: check for overlong encodings
Le tridi 13 fructidor, an CCXXII, Stefano Sabatini a écrit : > It is mathematically impossible that the length will be larger than 5. > Added an assert for that though, alternatively I could add a check. In ffprobe, the function is used with values coming directly from the file's metadata: an assert is not acceptable in this case. Furthermore, the function is capable of decoding the full UTF-8 range, up to (1<<31)-1, and that takes 6 octets. Also, I suspect checking for overlong encodings could have a flag just like the other extraneous checks below. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavu/avstring: check for overlong encodings
On date Saturday 2014-08-30 15:06:14 +0200, Nicolas George encoded: > Le tridi 13 fructidor, an CCXXII, Stefano Sabatini a écrit : > > It is mathematically impossible that the length will be larger than 5. > > Added an assert for that though, alternatively I could add a check. > > In ffprobe, the function is used with values coming directly from the file's > metadata: an assert is not acceptable in this case. Unless there is an error in the code, a tail length of 6 bytes should never be reached. > Furthermore, the function is capable of decoding the full UTF-8 range, up to > (1<<31)-1, and that takes 6 octets. There is a separate check in the function: if (code > 0x10 && !(flags & AV_UTF8_FLAG_ACCEPT_INVALID_BIG_CODES)) ret = AVERROR(EILSEQ); > Also, I suspect checking for overlong encodings could have a flag just like > the other extraneous checks below. I believe overlong encodings are illegal, and thus should be never accepted. -- FFmpeg = Formidable and Free Mastering Political Evil Gorilla ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavu/avstring: check for overlong encodings
Le tridi 13 fructidor, an CCXXII, Stefano Sabatini a écrit : > Unless there is an error in the code, a tail length of 6 bytes should > never be reached. > > > Furthermore, the function is capable of decoding the full UTF-8 range, up to > > (1<<31)-1, and that takes 6 octets. > > There is a separate check in the function: Sorry, I missed the fact that tail_len does not count the initial octet. The assert should be ok then. > I believe overlong encodings are illegal, and thus should be never > accepted. As you wish, a flag can be added later anyway if the default is to reject. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavu/avstring: check for overlong encodings
On date Saturday 2014-08-30 15:30:02 +0200, Nicolas George encoded: > Le tridi 13 fructidor, an CCXXII, Stefano Sabatini a écrit : > > Unless there is an error in the code, a tail length of 6 bytes should > > never be reached. > > > > > Furthermore, the function is capable of decoding the full UTF-8 range, up > > > to > > > (1<<31)-1, and that takes 6 octets. > > > > There is a separate check in the function: > > Sorry, I missed the fact that tail_len does not count the initial octet. The > assert should be ok then. > > > I believe overlong encodings are illegal, and thus should be never > > accepted. > > As you wish, a flag can be added later anyway if the default is to reject. Thanks for the reviews, finally pushed. -- FFmpeg = Faithless & Frightening Meaningless Philosophical Ecumenical Gem ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
