Xapian devel - Apr 2008 - [Xapian-commits] 10413: trunk/xapian-maintainer-tools/win32msvc/makedepend/

On Tue, Apr 29, 2008 at 05:30:09PM +0100, richard wrote:
> SVN root:       svn://svn.xapian.org/xapian
> Changes by:     richard
> Revision:       10413
> Date:           2008-04-29 17:30:09 +0100 (Tue, 29 Apr 2008)
> 
> Log message (1 line):
> Fix to reduce warnings.
> 
> Modified files:
> U   trunk/xapian-maintainer-tools/win32msvc/makedepend/ChangeLog
> U   trunk/xapian-maintainer-tools/win32msvc/makedepend/def.h
Um, how does this "reduce warnings"?

Unless I'm missing some subtlety, it just redefines BUFSIZ in an attempt
to make a buffer overflow less likely.  But that doesn't really address
the problem, does it?  It just means that you need to set a longer
include path to trigger it.  At the very minimum we really should do two
things as well as increasing the size of the buffer:

(a) Use a different define to BUFSIZ, which has a meaning in ISO C.
It's bad to just redefine it.

(b) Actually check that the buffer doesn't overflow by checking
against its fixed size before copying/appending to it.

I don't have MSVC, so I'd have to make such changes blind - it would
be better for someone who can test them to make them.
> http://xapian.org/C?10414?trunk/xapian-maintainer-tools/win32msvc/README
Since the buffer overflows still seem to be present, I'm going to
restore the warning about them unless someone can provide a good reason
not to (such as an explanation of which change actually fixed them, or a
patch to make sure they really can't happen).

I'm not at all happy that we're still knowingly shipping such shoddy
code (albeit in a build system utility), but while we still are, we owe
it to our users not to try to hide the fact.

Cheers,
    Olly

Olly Betts wrote:
> Unless I'm missing some subtlety, it just redefines BUFSIZ in an
attempt
> to make a buffer overflow less likely.  But that doesn't really address
> the problem, does it?  It just means that you need to set a longer
> include path to trigger it.  At the very minimum we really should do two
> things as well as increasing the size of the buffer:
> 
> (a) Use a different define to BUFSIZ, which has a meaning in ISO C.
> It's bad to just redefine it.
> 
> (b) Actually check that the buffer doesn't overflow by checking
> against its fixed size before copying/appending to it.
I've just committed fixes from Charlie for both of these.

-- 
Richard