Re: [Opengrads-devel] Buffer overflow vulnerability in GrADS

[Arlindo da S. <arl...@gm...>]

On Mon, Oct 26, 2009 at 4:43 PM, Jennifer Adams <jm...@co...> wrote:

> Hi, Arlindo --
> This is definitely worth fixing, but I think it will be simpler and faster
> to replace sprintf() with snprintf()
>

This is a 2 line fix: one call to snprintf(), followed  by ensuring NULL
termination, say pout[511]='\0'. An alternative would be to write a
slprintf() which does this combination (there are already strlcat(),
strlcpy() functions but no slprintf). However, folding this into gaprnt()
would allows us to simplify the code.


> rather than re-implement the message handling in GrADS.
>

You do not need to reimplement gaprnt(); it stays as it is. You just write a
gaprntf() cover function which does the pout thing with snprintf() and then
call the regular gaprnt(). Throughout the grads codebase you would be able
to replace the sprintf()+gaprnt() combination with a single call to
gaprntf(), therefore reducing loc.


> I'll put this on my list for the next release. I don't think it represents
> a security threat with the GDS.
>

I can think of a scenario or two where it could be exploited, but this is
not something I would broadcast on a publicly readable list. Probability is
low, though.

The main risk is that the whole gds+grads binary will not pass one of these
automated security audits, and the IT security cops would force us to take
the system down. Very often, you cannot argue with these guys.

   Thank you,

       Arlindo


--Jennifer
>
>
> On Oct 26, 2009, at 11:33 AM, Arlindo da Silva wrote:
>
> Brian, Jennifer et al,
>
>   As we all know, GrADS makes extensive use of the sprintf() function which
> is known to have the so-called buffer overflow vulnerability as explained in
> this document:
>
> http://developer.apple.com/mac/library/documentation/Security/Conceptual/SecureCodingGuide/Articles/BufferOverflows.html#//apple_ref/doc/uid/TP40002577
>
>   A good precaution would be to replace all occurrences of sprintf() with
> snprintf(), making sure the resulting string is NULL terminated.  Since many
> occurrences of sprintf() are associated with gaprnt(), it might be
> convenient to use the stdarg.h feature in the C standard library
>
>    http://en.wikipedia.org/wiki/Stdarg.h
>
>  and have a new function
>
>     gaprntf(int level,  const char *format, ...)
>
> which has the combined effect of sprintf() + gaprnt().
>
>   What do you think? I need to address this vulnerability before being able
> to deploy grads server side. I am willing to help implement this change in
> the main grads codebase.
>
>   Thanks,
>
>        Arlindo
>
>
> --
> Arlindo da Silva
> da...@al...
> ------------------------------------------------------------------------------
> Come build with us! The BlackBerry(R) Developer Conference in SF, CA
> is the only developer event you need to attend this year. Jumpstart your
> developing skills, take BlackBerry mobile applications to market and stay
> ahead of the curve. Join us from November 9 - 12, 2009. Register now!
>
> http://p.sf.net/sfu/devconference_______________________________________________
> Opengrads-devel mailing list
> Ope...@li...
> https://lists.sourceforge.net/lists/listinfo/opengrads-devel
>
>
> --
> Jennifer M. Adams
> IGES/COLA
> 4041 Powder Mill Road, Suite 302
> Calverton, MD 20705
> jm...@co...
>
>
>
>
>
> ------------------------------------------------------------------------------
> Come build with us! The BlackBerry(R) Developer Conference in SF, CA
> is the only developer event you need to attend this year. Jumpstart your
> developing skills, take BlackBerry mobile applications to market and stay
> ahead of the curve. Join us from November 9 - 12, 2009. Register now!
> http://p.sf.net/sfu/devconference
> _______________________________________________
> Opengrads-devel mailing list
> Ope...@li...
> https://lists.sourceforge.net/lists/listinfo/opengrads-devel
>
>


-- 
Arlindo da Silva
da...@al...