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.
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?
Adios!
--
Serge Koksharov, Free Software user & supporter
GPG public key ID: 0x3D330896 (pgp.mit.edu)
Key fingerprint: 5BC4 0475 CB03 6A31 0076 82C2 C240 72F0 3D33 0896
diff -Naur fvwmCVS-orig/bin/ChangeLog fvwmCVS-fixed/bin/ChangeLog
--- fvwmCVS-orig/bin/ChangeLog 2006-04-04 02:21:28.000000000 +0400
+++ fvwmCVS-fixed/bin/ChangeLog 2006-04-04 02:23:38.000000000 +0400
@@ -1,3 +1,8 @@
+2006-04-04 Serge Koksharov <gentoosiast dog yandex dot ru>
+
+ * ChangeLog:
+ corrected typo
+
2006-03-02 Serge Koksharov <gentoosiast dog yandex dot ru>
* fvwm-config.1.in:
@@ -59,7 +64,7 @@
2004-03-19 Dominik Vogt <dominik(dot)vogt(at)gmx(dot)de>
* fvwm-bug.in:
- script safty patch; see
+ script safety patch; see
http://securitytracker.com/alerts/2004/Jan/1008781.html
2004-01-01 Mikhael Goikhman <[EMAIL PROTECTED]>
diff -Naur fvwmCVS-orig/ChangeLog fvwmCVS-fixed/ChangeLog
--- fvwmCVS-orig/ChangeLog 2006-04-04 02:21:29.000000000 +0400
+++ fvwmCVS-fixed/ChangeLog 2006-04-04 02:36:25.000000000 +0400
@@ -1,3 +1,16 @@
+2006-04-04 Serge Koksharov <gentoosiast dog yandex dot ru>
+
+ * ChangeLog:
+ * bin/ChangeLog:
+ * libs/Fft.c:
+ * libs/System.c:
+ corrected typos
+
+ * libs/System.c (fvwm_mkstemp):
+ because of typo in the 'ifdef' pragma underlying OS's 'mkstemp'
+ function was never used, even if it was considered secure by configure
+ script.
+
2006-03-27 Dominik Vogt <dominik(dot)vogt(at)gmx(dot)de>
* fvwm/menus.c (pop_menu_up):
@@ -2374,7 +2387,7 @@
2003-08-07 olicha <[EMAIL PROTECTED]>
* configure.in:
- * acinclude.m4 (AM_SAFTY_CHECK_MKSTEMP):
+ * acinclude.m4 (AM_SAFETY_CHECK_MKSTEMP):
* acconfig.h:
* libs/System.c (fvwm_mkstemp):
* libs/fvwmlib.h:
diff -Naur fvwmCVS-orig/libs/Fft.c fvwmCVS-fixed/libs/Fft.c
--- fvwmCVS-orig/libs/Fft.c 2006-04-04 02:21:28.000000000 +0400
+++ fvwmCVS-fixed/libs/Fft.c 2006-04-04 02:24:38.000000000 +0400
@@ -324,7 +324,7 @@
{
goto bail;
}
- /* safty check */
+ /* safety check */
if (FftPatternGetMatrix(
load_pat, FFT_MATRIX, 0, &a) == FftResultMatch && a)
{
@@ -357,7 +357,7 @@
}
}
}
- /* FIXME: other safty checking ? */
+ /* FIXME: other safety checking ? */
fftfont = FftFontOpenPattern(dpy, load_pat);
if (!fftfont)
diff -Naur fvwmCVS-orig/libs/System.c fvwmCVS-fixed/libs/System.c
--- fvwmCVS-orig/libs/System.c 2006-04-04 02:21:28.000000000 +0400
+++ fvwmCVS-fixed/libs/System.c 2006-04-04 02:25:55.000000000 +0400
@@ -240,7 +240,7 @@
return *stamp != getFileStamp(name);
}
-#ifdef HAVE_SAFTY_MKSTEMP
+#ifdef HAVE_SAFETY_MKSTEMP
int fvwm_mkstemp (char *TEMPLATE)
{
return mkstemp(TEMPLATE);
@@ -328,4 +328,4 @@
__set_errno (EEXIST);
return -1;
}
-#endif /* HAVE_SAFTY_MKSTEMP */
+#endif /* HAVE_SAFETY_MKSTEMP */
