On 2012-11-30 19:56:14 +0100, Janne Grunau wrote:
> This prevents undefined behaviour of signed left shift if the coded
> value is larger than 2^31. Large values are most likely invalid and
> caused errors or by feeding random.
> 
> Validate every use of svq3_get_ue_golomb() and changed the place there
> the return value was compared with negative numbers. dirac.c was clean,
> fixed rv30 and svq3.
> ---
>  libavcodec/golomb.h |  5 +++--
>  libavcodec/rv30.c   |  6 +++---
>  libavcodec/svq3.c   | 17 ++++++++---------
>  3 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> index 6f95a67..564ba4e 100644
> --- a/libavcodec/golomb.h
> +++ b/libavcodec/golomb.h
> @@ -107,7 +107,8 @@ static inline int get_ue_golomb_31(GetBitContext *gb){
>      return ff_ue_golomb_vlc_code[buf];
>  }
>  
> -static inline int svq3_get_ue_golomb(GetBitContext *gb){
> +static inline unsigned svq3_get_ue_golomb(GetBitContext *gb)
> +{
>      uint32_t buf;
>  
>      OPEN_READER(re, gb);
> @@ -121,7 +122,7 @@ static inline int svq3_get_ue_golomb(GetBitContext *gb){
>  
>          return ff_interleaved_ue_golomb_vlc_code[buf];
>      }else{
> -        int ret = 1;
> +        unsigned ret = 1;
>  
>          do {
>              buf >>= 32 - 8;
> diff --git a/libavcodec/rv30.c b/libavcodec/rv30.c
> index 8016ad3..e4f3251 100644
> --- a/libavcodec/rv30.c
> +++ b/libavcodec/rv30.c
> @@ -73,7 +73,7 @@ static int rv30_decode_intra_types(RV34DecContext *r, 
> GetBitContext *gb, int8_t
>  
>      for(i = 0; i < 4; i++, dst += r->intra_types_stride - 4){
>          for(j = 0; j < 4; j+= 2){
> -            int code = svq3_get_ue_golomb(gb) << 1;
> +            unsigned code = svq3_get_ue_golomb(gb) << 1;
>              if(code >= 81*2){
>                  av_log(r->s.avctx, AV_LOG_ERROR, "Incorrect intra prediction 
> code\n");
>                  return -1;
> @@ -101,9 +101,9 @@ static int rv30_decode_mb_info(RV34DecContext *r)
>      static const int rv30_b_types[6] = { RV34_MB_SKIP, RV34_MB_B_DIRECT, 
> RV34_MB_B_FORWARD, RV34_MB_B_BACKWARD, RV34_MB_TYPE_INTRA, 
> RV34_MB_TYPE_INTRA16x16 };
>      MpegEncContext *s = &r->s;
>      GetBitContext *gb = &s->gb;
> -    int code = svq3_get_ue_golomb(gb);
> +    unsigned code     = svq3_get_ue_golomb(gb);
>  
> -    if (code < 0 || code > 11) {
> +    if (code > 11) {
>          av_log(s->avctx, AV_LOG_ERROR, "Incorrect MB type code\n");
>          return -1;
>      }
> diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c
> index ac8d9c1..4f0c2c0 100644
> --- a/libavcodec/svq3.c
> +++ b/libavcodec/svq3.c
> @@ -216,17 +216,15 @@ static inline int svq3_decode_block(GetBitContext *gb, 
> DCTELEM *block,
>      static const uint8_t *const scan_patterns[4] =
>      { luma_dc_zigzag_scan, zigzag_scan, svq3_scan, chroma_dc_scan };
>  
> -    int run, level, sign, vlc, limit;
> +    int run, level, limit;
> +    unsigned vlc;
>      const int intra           = 3 * type >> 2;
>      const uint8_t *const scan = scan_patterns[type];
>  
>      for (limit = (16 >> intra); index < 16; index = limit, limit += 8) {
>          for (; (vlc = svq3_get_ue_golomb(gb)) != 0; index++) {
> -            if (vlc == INVALID_VLC)
> -                return -1;
> -
> -            sign = (vlc & 0x1) - 1;
> -            vlc  = vlc + 1 >> 1;
> +            int sign = (vlc & 1) ? 0 : -1;
> +            vlc      = vlc + 1 >> 1;
>  
>              if (type == 3) {
>                  if (vlc < 3) {
> @@ -786,7 +784,7 @@ static int svq3_decode_slice_header(AVCodecContext *avctx)
>          skip_bits_long(&s->gb, 0);
>      }
>  
> -    if ((i = svq3_get_ue_golomb(&s->gb)) == INVALID_VLC || i >= 3) {
> +    if ((i = svq3_get_ue_golomb(&s->gb)) >= 3) {
>          av_log(h->s.avctx, AV_LOG_ERROR, "illegal slice type %d \n", i);
>          return -1;
>      }
> @@ -1010,7 +1008,7 @@ static int svq3_decode_frame(AVCodecContext *avctx, 
> void *data,
>      H264Context *h     = &svq3->h;
>      MpegEncContext *s  = &h->s;
>      int buf_size       = avpkt->size;
> -    int m, mb_type;
> +    int m;
>  
>      /* special case for last picture */
>      if (buf_size == 0) {
> @@ -1093,6 +1091,7 @@ static int svq3_decode_frame(AVCodecContext *avctx, 
> void *data,
>  
>      for (s->mb_y = 0; s->mb_y < s->mb_height; s->mb_y++) {
>          for (s->mb_x = 0; s->mb_x < s->mb_width; s->mb_x++) {
> +            unsigned mb_type;
>              h->mb_xy = s->mb_x + s->mb_y * s->mb_stride;
>  
>              if ((get_bits_count(&s->gb) + 7) >= s->gb.size_in_bits &&
> @@ -1113,7 +1112,7 @@ static int svq3_decode_frame(AVCodecContext *avctx, 
> void *data,
>                  mb_type += 8;
>              else if (s->pict_type == AV_PICTURE_TYPE_B && mb_type >= 4)
>                  mb_type += 4;
> -            if ((unsigned)mb_type > 33 || svq3_decode_mb(svq3, mb_type)) {
> +            if (mb_type > 33 || svq3_decode_mb(svq3, mb_type)) {
>                  av_log(h->s.avctx, AV_LOG_ERROR,
>                         "error while decoding MB %d %d\n", s->mb_x, s->mb_y);
>                  return -1;

ping

Janne
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to