Kees Cook wrote: > On Fri, Mar 20, 2009 at 02:09:33PM -0000, Aurélien Gâteau wrote: >> http://launchpadlibrarian.net/24149888/02-avoid_potential_buffer_overrun.diff > > "n" should be verified to be !=0, so that result[-1] isn't used. [snip]
> This change does not have the same functionality (in toXMLStringUnSafe): [snip] > the original can copy up to 4 characters (notice the lack of breaks). > > Also, nothing in the loop is testing that "length" is remaining positive. > This should be checked in toXMLStringUnSafe, CreateXMLStringR (if nResult > is ever >= length, abort), and in _tcscpy (where it should abort if n<0). [snip] I agree with these remarks (shame on me for the switch() error), I went to fix those bugs, but after further inspection, I believe the changes are not necessary or at least not at the right place: _tcspy() is called from CreateXMLStringR() and toXMLStringUnSafe() only. - CreateXMLStringR() can be called with a null pointer, in this case it compute the necessary buffer size. This is what CreateXMLString(), the only caller, does. - toXMLStringUnSafe() is called from CreateXMLStringR(), which is fine and from toXML(), which compute the source string length before. All in all I think the code is fine, or that the fix should be on the malloc()/realloc() calls which are done after computing the buffer lengths. -- Include libmsn in main https://bugs.launchpad.net/bugs/308060 You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. -- ubuntu-bugs mailing list [email protected] https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
