[XBoard-devel] Re: sending security patches

[Chris Frey]

On Sun, Sep 04, 2005 at 11:57:43PM -0700, Tim Mann wrote:
> I think we'd be happy to get such patches.  However, the project hasn't
> been very active for a few years, so I can't promise someone will get
> around to merging them soon.
> 
> Hopefully they will be trivial and short enough that we can use them
> without having to get you to sign FSF paperwork.  That's actually not a
> huge deal, though.

Cool.


> One technical point: there are currently a lot of potential buffer
> overflows in the code because of sprintf'ing into a buffer with
> unchecked length (or the like).  However, just converting them all to
> snprintf (etc.) will still leave the program buggy -- silently
> truncating long inputs is better than corrupting memory, but what we
> really should do in many cases is either (a) accept arbitrarily long
> inputs or (b) generate an error message if the input is too long.

I'm not sure I have the time to do option (a), and especially not in
plain C. :-)

You know the code much better than I do, so what error handling for
option (b) would you suggest?  A callback?

For example, in the backend.c:looking_at() function, it copies wildcard
matches into the star_match array without checking the length.  A username
from the server greater than 512 bytes would overflow this buffer, and
others down the line.  Would you prefer the error to be reported
inside looking_at()?  That seems a little messy, but changing the return
code to a tristate would make error checking code balloon to huge
proportions. :-)

Thanks for your quick response,
- Chris