On 4/3/06, Serge (gentoosiast) Koksharov [EMAIL PROTECTED] wrote:
Good (day|morning|night) everyone,
During examination of FvwmM4 '--debug' option I decided to examine FVWM's
temporary file creation mechanism. Can you believe what I dig out:
In libs/System.c there is a pragma '#ifdef HAVE_SAFTY_MKSTEMP'. This
construction decides based on configure script system check whether to
use underlying OS's mkstemp function (if it considered secure) or FVWM's
internal one, which lies at the bottom of the same libs/System.c file.
But acinclude.m4 defines 'HAVE_SAFETY_MKSTEMP' pragma, not
'HAVE_SAFTY_MKSTEMP' which found in libs/System.c. So, in any case
FVWM's internal implementation of mkstemp used even if the OS have its
own _much more secure_ version of this function. This bug probably
existed for almost three years and was introduced on 2003-08-27
according to main Changelog. I attached patch which applies cleanly
against 2.5.x CVS sources. It also corrects all other 'safty' typos in
the source tree. Somebody on the list needs to verify stable 2.4 branch
also.
Wow! And there must be more, as the code is quite old..
This example shows that a single typo can potentially lead to the big
disaster. I hope it will be good lesson to all of us. In future, all
workers should review every commit more attentively. It's much easier to
not introduce newer bugs and typos than to find and fix them afterwards.
I wonder, was FVWM's code extensively audited? Who knows that may be
lurking inside?
I guess there should be a complete rewrite for the next major version,
but on the other hand there's a need to release a stable 2.6.. And
this last task is proving to be quite discouraging, as people with new
ideas tend to get bored when all they should do is to clean up the old
code.. There's a TODO list in the source that must (should) be done
before the next release.
Auditing the code may be a huge task, but it surely helps the next
release, and any help is wellcome! On the other hand it can be useless
in the iminence of a major rewrite, so if there is really going to be
one, maybe we should audit the code as we rewrite it, having the
errors corrected on the stable 2.6 version.
This is just my two cents, anyone is free to disagree..
Cheers!
Renato Caldas
