Re: [PATCH 4/6] winealsa.drv: Append hw address to handle to prevent name collision. (resend)

[Vitaliy Margolen]

On 08/02/2010 05:49 PM, Jeff Cook wrote:
> ---
> dlls/winealsa.drv/waveinit.c | 9 +++++++--
> 1 files changed, 7 insertions(+), 2 deletions(-)
> +    sprintf(description, "%s %d:%d", snd_pcm_info_get_name(infop), snd_pcm_info_get_card(infop),
> Please use snprintf to prevent possible buffer overflow.
> +    memcpy(ww->ds_desc.szDesc, description,
> +            min( (sizeof(ww->ds_desc.szDesc) - 1), strlen(description)) );
> This does not guarantee that ww->ds_desc.szDesc will be \0 terminated.

Vitaliy.

[PATCH 4/6] winealsa.drv: Append hw address to handle to prevent name collision. (resend)

[Jeff Cook]

On Tue, Aug 3, 2010 at 7:49 AM, Vitaliy Margolen
<wine-devel(a)kievinfo.com> wrote:
> > +    memcpy(ww->ds_desc.szDesc, description,
> > +            min( (sizeof(ww->ds_desc.szDesc) - 1), strlen(description))
> > );
>
> This does not guarantee that ww->ds_desc.szDesc will be \0 terminated.
>
> Vitaliy.

What do you want me to do? This looks pretty safe to me and I just
copied the block from elsewhere and replaced the variables. Do you
want me to manually read to the end of ww->ds_desc.szDesc and replace
with a \0? Sorry for the noobness.

From
Jeff

[PATCH 4/6] winealsa.drv: Append hw address to handle to prevent name collision. (resend)

[Vitaliy Margolen]

On 08/29/2010 12:43 AM, Jeff Cook wrote:
> On Tue, Aug 3, 2010 at 7:49 AM, Vitaliy Margolen
> <wine-devel(a)kievinfo.com>  wrote:
> > > +    memcpy(ww->ds_desc.szDesc, description,
> > > +            min( (sizeof(ww->ds_desc.szDesc) - 1), strlen(description))
> > > );
> >
> > This does not guarantee that ww->ds_desc.szDesc will be \0 terminated.
> >
> > Vitaliy.
>
> What do you want me to do? This looks pretty safe to me and I just
> copied the block from elsewhere and replaced the variables. Do you
> want me to manually read to the end of ww->ds_desc.szDesc and replace
> with a \0? Sorry for the noobness.

strlen returns number of characters without terminating \0. If you using 
memcpy to copy strings you must add additional byte (if destination is big 
enough). Or explicitly zero terminate the destination string (if result was 
truncated).

What you really want is a strlcpy() which unfortunately not present on all 
platforms. And don't even think using strncpy - it's the worst string 
function ever made.

Just the fact that you copied the exact code doesn't automatically make it 
correct.

Vitaliy.