On Tue, Oct 06, 2015 at 11:57:40PM +0200, Tobias Stoeckmann wrote:
> By the way, this is the second version with miod's feedback. Time to
> send it to tech@ now, too.
> 
> Fixed one issue due to missing braces and less ntohl() calls, which
> makes the code easier to read.

ok with me

> Index: catopen.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/nls/catopen.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 catopen.c
> --- catopen.c 5 Sep 2015 11:25:30 -0000       1.17
> +++ catopen.c 14 Sep 2015 18:27:00 -0000
> @@ -30,20 +30,24 @@
>  
>  #define _NLS_PRIVATE
>  
> -#include <limits.h>
> -#include <stdlib.h>
> -#include <string.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
>  #include <sys/mman.h>
> -#include <unistd.h>
> +#include <errno.h>
>  #include <fcntl.h>
> +#include <limits.h>
>  #include <nl_types.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#define MAXIMUM(a, b)        (((a) > (b)) ? (a) : (b))
>  
>  #define NLS_DEFAULT_PATH 
> "/usr/share/nls/%L/%N.cat:/usr/share/nls/%l.%c/%N.cat:/usr/share/nls/%l/%N.cat"
>  #define NLS_DEFAULT_LANG "C"
>  
> -static nl_catd load_msgcat(const char *);
> +static nl_catd       load_msgcat(const char *);
> +static int   verify_msgcat(nl_catd);
>  
>  nl_catd
>  catopen(const char *name, int oflag)
> @@ -165,6 +169,8 @@ load_msgcat(const char *path)
>       void *data;
>       int fd;
>  
> +     catd = NULL;
> +
>       if ((fd = open(path, O_RDONLY|O_CLOEXEC)) == -1)
>               return (nl_catd) -1;
>  
> @@ -173,24 +179,106 @@ load_msgcat(const char *path)
>               return (nl_catd) -1;
>       }
>  
> -     data = mmap(0, (size_t) st.st_size, PROT_READ, MAP_SHARED, fd, 
> (off_t)0);
> -     close (fd);
> -
> -     if (data == MAP_FAILED) {
> +     if (st.st_size > INT_MAX || st.st_size < sizeof (struct _nls_cat_hdr)) {
> +             errno = EINVAL;
> +             close (fd);
>               return (nl_catd) -1;
>       }
>  
> -     if (ntohl(((struct _nls_cat_hdr *) data)->__magic) != _NLS_MAGIC) {
> -             munmap(data, (size_t) st.st_size);
> -             return (nl_catd) -1;
> -     }
> +     data = mmap(0, (size_t)st.st_size, PROT_READ, MAP_SHARED, fd, (off_t)0);
> +     close (fd);
>  
> -     if ((catd = malloc(sizeof (*catd))) == 0) {
> -             munmap(data, (size_t) st.st_size);
> +     if (data == MAP_FAILED)
>               return (nl_catd) -1;
> -     }
> +
> +     if (ntohl(((struct _nls_cat_hdr *) data)->__magic) != _NLS_MAGIC)
> +             goto invalid;
> +
> +     if ((catd = malloc(sizeof (*catd))) == 0)
> +             goto invalid;
>  
>       catd->__data = data;
>       catd->__size = st.st_size;
> +
> +     if (verify_msgcat(catd))
> +             goto invalid;
> +
>       return catd;
> +
> +invalid:
> +     free(catd);
> +     munmap(data, (size_t) st.st_size);
> +     errno = EINVAL;
> +     return (nl_catd) -1;
>  }
> +
> +static int
> +verify_msgcat(nl_catd catd)
> +{
> +     struct _nls_cat_hdr *cat;
> +     struct _nls_set_hdr *set;
> +     struct _nls_msg_hdr *msg;
> +     size_t remain;
> +     int hdr_offset, i, index, j, msgs, nmsgs, nsets, off, txt_offset;
> +
> +     remain = catd->__size;
> +     cat = (struct _nls_cat_hdr *) catd->__data;
> +
> +     hdr_offset = ntohl(cat->__msg_hdr_offset);
> +     nsets = ntohl(cat->__nsets);
> +     txt_offset = ntohl(cat->__msg_txt_offset);
> +
> +     /* catalog must contain at least one set and no negative offsets */
> +     if (nsets < 1 || hdr_offset < 0 || txt_offset < 0)
> +             return (1);
> +
> +     remain -= sizeof (*cat);
> +
> +     /* check if offsets or set size overflow */
> +     if (remain <= hdr_offset || remain <= ntohl(cat->__msg_txt_offset) ||
> +         remain / sizeof (*set) < nsets)
> +             return (1);
> +
> +     set = (struct _nls_set_hdr *) ((char *) catd->__data + sizeof (*cat));
> +
> +     /* make sure that msg has space for at least one index */
> +     if (remain - hdr_offset < sizeof(*msg))
> +             return (1);
> +
> +     msg = (struct _nls_msg_hdr *) ((char *) catd->__data + sizeof (*cat)
> +         + hdr_offset);
> +
> +     /* validate and retrieve largest string offset from sets */
> +     off = 0;
> +     for (i = 0; i < nsets; i++) {
> +             index = ntohl(set[i].__index);
> +             nmsgs = ntohl(set[i].__nmsgs);
> +             /* set must contain at least one message */
> +             if (index < 0 || nmsgs < 1)
> +                     return (1);
> +
> +             if (INT_MAX - nmsgs < index)
> +                     return (1);
> +             msgs = index + nmsgs;
> +
> +             /* avoid msg index overflow */
> +             if ((remain - hdr_offset) / sizeof(*msg) < msgs)
> +                     return (1);
> +
> +             /* retrieve largest string offset */
> +             for (j = index; j < nmsgs; j++) {
> +                     if (ntohl(msg[j].__offset) < 0)
> +                             return (1);
> +                     off = MAXIMUM(off, ntohl(msg[j].__offset));
> +             }
> +     }
> +
> +     /* check if largest string offset is nul-terminated */
> +     if (remain - txt_offset < off ||
> +         memchr((char *) catd->__data + sizeof(*cat) + txt_offset + off,
> +         '\0', remain - txt_offset - off) == NULL)
> +             return (1);
> +
> +     return (0);
> +}
> +

Reply via email to