[mapserver-dev] Ticket 3537 - Buffer overflow vulnerabilities

[Alan Boudreault]

Thomas,

* I don't use sizeof() operator to compute string lengths, I use strlen to 
compute string length. If so, it's a mistake. snprintf need the total size of 
the buffer we want to write in. So, it's not the lenght of the string I need, 
but its size. Let me know if that's not what you are refering to.

* I assume you are talking about the msSaveImageAGG() function etc. Well, in 
that case, it was not risky (and there are a few other places like that) but I 
wasn't sure at 100% if I could say that the format->driver length passed in 
function parameter was smaller than 128. Can this function be called from php? 
or from mapscript? I couldn't really do that for all function I modified, it 
would have taken too many time. Also, I don't see any reason to not use those 
functions since they are just safer. Btw, there are many cases that I haven't 
modified the strcpy calls.... when I considered them safe.

Do I answer your questions? 

Thanks,
Alan

On October 6, 2010 08:49:54 am thomas bonfort wrote:
> Alan,
> * it seems suspicious to me that you are using the sizeof operator to
> compute string lengths,shouldn't you be using strlen instead?
> * why replace all functions with their snyyyy couterpart (i.e. sprintf
> by snprintf, etc...) when treating data that isn't submitted over the
> wire (for example DRIVER names, etc...) ?
> 
> regards,
> thomas
> 
> On Tue, Oct 5, 2010 at 19:57, Alan Boudreault <aboudreault at mapgears.com> 
wrote:
> > Hi Devs,
> > 
> > As discussed during the meeting at FOSS4G 2010, I passed through the
> > mapserver code source and fixed a lot buffer overflow vulnerabilities. I
> > followed the good practices in C development of a few security sites. ie:
> > https://buildsecurityin.us-cert.gov/bsi-rules/home.html
> > 
> > I invite all file maintainers to take a look at my changes to see what
> > those good practices are and comment if needed. If you have no
> > objection, I'm going to commit this in trunk.
> > 
> > I've run msautotest and the results before/after applying those patches
> > are exactly the same. I would like to commit as soon as possible to let
> > everyone test their applications with the changes.
> > 
> > Here's the patches:
> > 
> > http://trac.osgeo.org/mapserver/attachment/ticket/3537/3537-1.patch
> > 
> > http://trac.osgeo.org/mapserver/attachment/ticket/3537/3537-2.patch
> > 
> > regards,
> > 
> > Alan
> > 
> > --
> > 
> > Alan Boudreault
> > 
> > Mapgears
> > 
> > http://www.mapgears.com
> > 
> > _______________________________________________
> > mapserver-dev mailing list
> > mapserver-dev at lists.osgeo.org
> > http://lists.osgeo.org/mailman/listinfo/mapserver-dev

-- 
Alan Boudreault
Mapgears
http://www.mapgears.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osgeo.org/pipermail/mapserver-dev/attachments/20101006/c98f366c/attachment-0001.html