[PATCH] d3dxof: avoid some strcpy overflows (Coverity)

[Henri Verbeet]

On 4 September 2016 at 11:24, Marcus Meissner <marcus at jet.franken.de> wrote:
> @@ -532,8 +532,11 @@ static BOOL is_name(parse_buffer* buf)
>    buf->rem_bytes -= pos;
>
>    TRACE("Found name %s\n", tmp);
> +  if (strlen(tmp)+1 > sizeof(buf->value)) {
> +    FIXME("name %s exceeds buffer length\n", tmp);
> +    return FALSE;
> +  }
>    strcpy((char*)buf->value, tmp);
> -
>    return TRUE;
>  }
There are plenty of things wrong with d3dxof, but assuming the
preceding code isn't too broken, I don't think this can happen. You
should probably just get rid of the "tmp" buffer and use read_bytes()
or something similar after calculating "pos" though. (The TRACE can
use debugstr_an().) That probably applies to is_string() as well.

> @@ -928,6 +937,10 @@ static BOOL parse_template_option_info(parse_buffer * buf)
>      {
>        if (get_TOKEN(buf) != TOKEN_NAME)
>          return FALSE;
> +      if (strlen((char*)buf->value)+1 > sizeof(cur_template->children[cur_template->nb_children])) {
> +        FIXME("name %s too long for buffer\n", (char*)buf->value);
> +        return FALSE;
> +      }
>        strcpy(cur_template->children[cur_template->nb_children], (char*)buf->value);
Once you've calculated to number of bytes you need to copy anyway, you
might as well just use memcpy(). I.e., strcpy() is almost always the
wrong choice, for one reason or another.