headers
Chris Frey | 5 Sep 2005 08:28
Picon
Favicon

sending security patches

Hi,

I'm just checking whether it would be welcomed if I posted security
patches to the list as I found them.

For example, if I went through and changed all sprintf and strcpy/strcat
calls to their safer equivalents, would that be accepted?

If I posted them to the list with a public domain notice, could they
go into CVS right away?

Thanks for any feedback,
- Chris
Permalink | Reply | 4 comments |
headers
Tim Mann | 5 Sep 2005 08:57

Re: sending security patches

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.

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.

On Mon, 5 Sep 2005 02:28:51 -0400, Chris Frey <cdfrey <at> netdirect.ca> wrote:
> Hi,
> 
> I'm just checking whether it would be welcomed if I posted security
> patches to the list as I found them.
> 
> For example, if I went through and changed all sprintf and strcpy/strcat
> calls to their safer equivalents, would that be accepted?
> 
> If I posted them to the list with a public domain notice, could they
> go into CVS right away?
> 
> Thanks for any feedback,
> - Chris
(Continue reading)

Permalink | Reply | 3 comments |
headers
Chris Frey | 5 Sep 2005 09:33
Picon
Favicon

Re: sending security patches

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
(Continue reading)

Permalink | Reply | 2 comments |
headers
Tim Mann | 5 Sep 2005 23:23

Re: Re: sending security patches

On Mon, 5 Sep 2005 03:33:21 -0400, Chris Frey <cdfrey <at> netdirect.ca> 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
(Continue reading)

Permalink | Reply | 1 comment |
headers
Chris Frey | 6 Sep 2005 09:01
Picon
Favicon

Re: Re: sending security patches

On Mon, Sep 05, 2005 at 02:23:35PM -0700, Tim Mann wrote:
> 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.

Hmmm, that could work too.

I'm also pondering adding some SendToProgramf() type functions, which handle
all formatting just like sprintf(), but keeps the buffer code in only one
spot.

Well, I'll post whatever good patches I manage to cook up.  Thanks!

- Chris
Permalink | Reply | 0 comments |

Gmane