Re: [XBoard-devel] Re: sending security patches

[Tim Mann]

On Mon, 5 Sep 2005 03:33:21 -0400, Chris Frey <address@hidden> wrote:
> 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. :-)

I haven't really given it much thought, but I agree that adding detailed
error checking to all the places that use fixed-size buffers now would
blow things up.  The most I'd really hope for is that if there's a place
where it makes sense to do something other than truncating the output to
fit the buffer, we do that.  

There may be a couple of places where buffers should be bigger, but I
think Daniel attacked the worst of those.  The one I remember was the
line buffer to read output from a chess engine, since an engine can
print "thinking" output with arbitrarily long PVs.  Some engine authors
really hit this one.

Possibly there are also places where one could easily compute the buffer
size needed and dynamically allocate it, but then you have to find the
right place to free it, which won't always be trivial.  Sigh.

In the case of looking_at, maybe it should just fail to match if
something that would match "*" is too long.  It probably means that a
pattern matched somewhere it shouldn't have.  Then again, maybe it's not
that much better than truncating a long * match.


> 
> _______________________________________________
> XBoard-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/xboard-devel
> 


-- 
Tim Mann  address@hidden  http://tim-mann.org/