"Sander Striker" <[EMAIL PROTECTED]> writes:

> Jeff,
> 
> Does this resolve the issue you added the comment for?

no

1) the strncpy() can go beyond the allocated storage for the buffer
2) no terminating '\0' for the copy of the tag, so strstr() can't work
   anyway 

> Index: modules/mappers/mod_negotiation.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_negotiation.c,v
> retrieving revision 1.96
> diff -u -r1.96 mod_negotiation.c
> --- modules/mappers/mod_negotiation.c   12 Mar 2002 11:48:32 -0000      1.96
> +++ modules/mappers/mod_negotiation.c   12 Mar 2002 12:20:01 -0000
> @@ -794,8 +794,12 @@
>  {
>      char *endbody;
>      int bodylen;
> +    int taglen;
>      apr_off_t pos;
> 
> +    taglen = strlen(tag);
> +    *len -= taglen;
> +
>      /* We are at the first character following a body:tag\n entry
>       * Suck in the body, then backspace to the first char after the
>       * closing tag entry.  If we fail to read, find the tag or back
> @@ -803,13 +807,11 @@
>       */
>      if (apr_file_read(map, buffer, len) != APR_SUCCESS) {
>          return -1;
> -    }
> -    /* XXX next line can go beyond allocated storage and segfault,
> -     *     or worse yet go beyond data read but not beyond allocated
> -     *     storage and think it found the tag
> -     */
> +    }
> +
> +    strncpy(buffer + *len, tag, taglen);
>      endbody = strstr(buffer, tag);
> -    if (!endbody) {
> +    if (!endbody || endbody == buffer + *len) {
>          return -1;
>      }
>      bodylen = endbody - buffer;
> 

-- 
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...

Reply via email to