[BUG] A string format defect in Nginx 0.8.48

[Gary Hu]

Hi Maxim,

Thanks for your comments.  I thought that "%*s" in the comments above
function "ngx_sprintf" is meant to be used for right alignment.  And it
turns out to be my own misunderstanding.

Gary

On Tue, Aug 10, 2010 at 5:07 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:

> Hello!
>
> On Tue, Aug 10, 2010 at 12:07:30PM +0800, Gary Hu wrote:
>
> > I found a defect in function "ngx_vslprintf”which doesn't support right
> > alignment format of a string, such as "%40s", as purported in the
> comments
> > right above the function "ngx_sprintf" in file "src/core/ngx_string.c".
>
> It is not clear why you think it should be supported and no
> support is a bug.  Both comments and code agree that it's not
> supported.
>
> Additionally, it's not clear why do you think it should be
> supported for %s, but not for %*s, %V and %v.
>
> And some patch comments just for completeness:
>
> > The following is my suggested fix in function "ngx_vslprintf ":
> >
> >
> >
> >             ......
> >
> >            case 's':
> >
> >               p = va_arg(args, u_char *);
> >
> >
> >
> >                 //
> >
> >                 // Added  by Gary Hu
> >
> >                 //
>
> You may want to use diff -u in the future for submitting patches.
>
> >                 size_t sublen;
> >
> >                 size_t padlen;
>
> Please follow nginx style when submitting patches.  In particular,
> please avoid declaring variables in function body.  And note that
> nobody stops you from reusing len for padlen, and slen for sublen.
> So basically declared variables aren't needed.
>
> >
> >                 sublen = ngx_strlen(p);
>
> This will likely cause SIGSEGV/SIGBUS for %*s as p isn't null
> terminated in this case.
>
> >
> >
> >
> >                 /* Add padding whitespaces to the output buffer to
> achieve
> > right alignment*/
> >
> >                 if (width > 0 && width > sublen)
>
> Check "width > 0" is useless as both width and sublen are
> unsigned.
>
> >                 {
> >
> >                     padlen = ngx_min(((size_t) (last - buf)), width -
> sublen
> > );
> >
> >                     ngx_memset(buf, ' ', padlen);
> >
> >                     buf += padlen;
> >
> >                 }
> >
> >                 //
> >
> >                 // End added by Gary Hu
> >
> >                 //
>
> Maxim Dounin
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://nginx.org/mailman/listinfo/nginx-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://nginx.org/pipermail/nginx-devel/attachments/20100810/6a06b51e/attachment.html>