Re: gdiplus: in GdipDrawString only NULL terminate if we have room in the buffer.

[Nikolay Sivov]

Aric Stewart wrote:
> ---
>  dlls/gdiplus/graphics.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c
> index 2673042..c276a43 100644
> --- a/dlls/gdiplus/graphics.c
> +++ b/dlls/gdiplus/graphics.c
> @@ -1901,7 +1901,8 @@ GpStatus WINGDIPAPI GdipDrawString(GpGraphics *graphics, GDIPCONST WCHAR *string
>          j++;
>      }
>
> -    stringdup[j] = 0;
> +    if (j < length)
> +        stringdup[j] = 0;
>      length = j;
>
> while(sum < length){

Hm, maybe I don't follow something, but I think 'j < length' is always 
true after that:
---
    for(i = 0, j = 0; i < length; i++){
        if(!isprintW(string[i]) && (string[i] != '\n'))
            continue;

        stringdup[j] = string[i];
        j++;
    }

    stringdup[j] = 0;
    length = j;
---
Or you thought about zero length case? It should be handled earlier, on 
stringdup allocation or before...

gdiplus: in GdipDrawString only NULL terminate if we have room in the buffer.

[Aric Stewart]

if we do this call with a string of "XX" and a length of 2.
then we create a 2 WCHAR buffer to store the string in.

then i == 0; j == 0 on the first look i++  and j++  so i == 1, j == 1 
and length is 2.

then we loop again and i++ and j++ so i == 2, j == 2 and length == 2.

so then when we do the stringdup[j] = 0  we are writing one WCHAR off 
the end of the buffer.

-aric

Nikolay Sivov wrote:
> Aric Stewart wrote:
> > ---
> >  dlls/gdiplus/graphics.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c
> > index 2673042..c276a43 100644
> > --- a/dlls/gdiplus/graphics.c
> > +++ b/dlls/gdiplus/graphics.c
> > @@ -1901,7 +1901,8 @@ GpStatus WINGDIPAPI GdipDrawString(GpGraphics *graphics, GDIPCONST WCHAR *string
> >          j++;
> >      }
> >
> > -    stringdup[j] = 0;
> > +    if (j < length)
> > +        stringdup[j] = 0;
> >      length = j;
> >
> > while(sum < length){
>
> Hm, maybe I don't follow something, but I think 'j < length' is always 
> true after that:
> ---
>     for(i = 0, j = 0; i < length; i++){
>         if(!isprintW(string[i]) && (string[i] != '\n'))
>             continue;
>
> stringdup[j] = string[i];
>         j++;
>     }
>
> stringdup[j] = 0;
>     length = j;
> ---
> Or you thought about zero length case? It should be handled earlier, on 
> stringdup allocation or before...