[FFmpeg-devel] [PATCH] avformat/pjsdec: check strcspn values before using them

[Clément Bœsch]

On Fri, Jan 10, 2014 at 02:05:47AM +0100, Michael Niedermayer wrote:
> Fixes use of uninitialized memory
> Fixes: msan_uninit-mem_7f91f2de7764_2649_PJS_capability_tester.pjs
> Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  libavformat/pjsdec.c |   17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/pjsdec.c b/libavformat/pjsdec.c
> index a69a316..00866b7 100644
> --- a/libavformat/pjsdec.c
> +++ b/libavformat/pjsdec.c
> @@ -65,6 +65,7 @@ static int pjs_read_header(AVFormatContext *s)
>      PJSContext *pjs = s->priv_data;
>      AVStream *st = avformat_new_stream(s, NULL);
>      int res = 0;
> +    int idx;
>  
>      if (!st)
>          return AVERROR(ENOMEM);
> @@ -83,13 +84,25 @@ static int pjs_read_header(AVFormatContext *s)
>          if (!len)
>              break;
>  
> -        line[strcspn(line, "\r\n")] = 0;
> +        idx = strcspn(line, "\r\n");
> +        if (!line[idx]) {
> +            av_log(s, AV_LOG_ERROR, "missing newline\n");
> +            return AVERROR_INVALIDDATA;
> +        }
> +
> +        line[idx] = 0;
>  
>          pts_start = read_ts(&p, &duration);
>          if (pts_start != AV_NOPTS_VALUE) {
>              AVPacket *sub;
>  
> -            p[strcspn(p, "\"")] = 0;
> +            idx = strcspn(p, "\"");
> +            if (!p[idx]) {
> +                av_log(s, AV_LOG_ERROR, "missing \"\n");
> +                return AVERROR_INVALIDDATA;
> +            }
> +
> +            p[idx] = 0;
>              sub = ff_subtitles_queue_insert(&pjs->q, p, strlen(p), 0);
>              if (!sub)
>                  return AVERROR(ENOMEM);

The use of strcspn() as such is fine (and we use it everywhere). I'd suggest
instead:

diff --git a/libavformat/pjsdec.c b/libavformat/pjsdec.c
index a69a316..6f5db37 100644
--- a/libavformat/pjsdec.c
+++ b/libavformat/pjsdec.c
@@ -53,7 +53,8 @@ static int64_t read_ts(char **line, int *duration)
     int64_t start, end;
 
     if (sscanf(*line, "%"SCNd64",%"SCNd64, &start, &end) == 2) {
-        *line += strcspn(*line, "\"") + 1;
+        *line += strcspn(*line, "\"");
+        *line += !!**line;
         *duration = end - start;
         return start;
     }

Which should be enough to enough to fix the problem.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140110/a5f3a0fc/attachment.asc>