[hylafax-devel] Re: Security patches for hylafax-v4.0pl2

[Arjan Knepper <arjan@jak.nl>]

Dmitry Bely wrote:
> 
> John Holland <john@zoner.org> writes:
> 
> > I had a bit of time last weekend, so in the interest of patching the setuid
> > uucp buffer overflow in faxalter and getting the FreeBSD/OpenBSD port
> > current, I did a beginnings of a security audit on hylafax-v4.0pl2.  Below
> > are patches for various unbounded string copies in the source.
> >
> > The functions that I checked are:
> >
> > strcpy
> > strcat
> > sprintf
> > vsprintf
> >
> > There were a few instances of possible unbounded character string copies
> > that I did not alter.  The fix was non-obvious and buried deep in support
> > functions.  I also did not check any possible race conditions.
> >
> > What is the status of hylafax-v4.0pl2?  Is any work being done on it?  Is
> > another patch level forthcoming?  Or is the effort being targeted to
> > 4.1beta?  Would my time be better spent looking at the latter?
> >
> >
> >
> >
> >
> > diff -cr ./faxalter/faxalter.c++
> > /usr/ports/comms/hylafax_new/hylafax-v4.0pl2/faxalter/faxalter.c++
> > *** ./faxalter/faxalter.c++   Sat Feb 14 05:48:38 1998
> > --- /usr/ports/comms/hylafax_new/hylafax-v4.0pl2/faxalter/faxalter.c++        Fri
> > Jun  9 15:01:00 2000
> > ***************
> > *** 185,191 ****
> >        va_list ap;
> >        va_start(ap, fmt0);
> >        char fmt[1024];
> > !     sprintf(fmt, "%s %s\n", groups ? "JGPARM" : "JPARM", fmt0);
> >        script.append(fxStr::vformat(fmt, ap));
> >        va_end(ap);
> >    }
> > --- 185,191 ----
> >        va_list ap;
> >        va_start(ap, fmt0);
> >        char fmt[1024];
> > !     snprintf(fmt, 1024, "%s %s\n", groups ? "JGPARM" : "JPARM", fmt0);
> 
> Frankly speaking, I don't like such fixes. First, snprintf-class functions
> are non-standard (they are GNU extensions), and so may not be present in
> any system. And second (more important) -- why use this ancient C-style
> string manipulation functions with endless buffer overruns in the modern
> C++ software? Better rewrite the code (not so much work) to use fxStr (or
> standard string class which is preferable) instead.

I agree with that, btw this is mentioned before.

IMHO it is better to spend some time learning how to this in C++
than start right away fixing this with acient C-style
programming. The standard C++ string classes are not that
difficult to understand.
 
> Hope to hear from you soon,
> Dmitry

Me too

Arjan Knepper