Hylafax Developers Mailing List Archives
|
[Date Prev][Date Next][Thread Prev][Thread Next]
[Date Index]
[Thread Index]
[hylafax-devel] Re: Security patches for hylafax-v4.0pl2
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