Since the security fix in bug #66400 was applied to ed, it no longer produces a correct output file if the stderr file description is closed. Instead it produces a corrupted output file. If the patch from #66400 is NOT used, then the bug does not occur. Reproducible: Always Steps to Reproduce: see attached test script files. Actual Results: if stderr is closed when ed runs, the output file is corrupted when the 'write' command is run in ed. Expected Results: The output file should not be corrupted. Portage 2.0.51-r3 (!/usr/portage/profiles/default-linux/x86/2004.3, gcc-3.3.5, glibc-2.3.4.20041102-r0, 2.6.7-mm4 i686) ================================================================= System uname: 2.6.7-mm4 i686 AMD Athlon(tm) XP 2400+ Gentoo Base System version 1.6.6 distcc 2.18 i686-pc-linux-gnu (protocols 1 and 2) (default port 3632) [enabled] ccache version 2.3 [enabled] Autoconf: sys-devel/autoconf-2.59-r5 Automake: sys-devel/automake-1.8.5-r1 Binutils: sys-devel/binutils-2.15.92.0.2-r1 Headers: sys-kernel/linux26-headers-2.6.8.1-r1 Libtools: sys-devel/libtool-1.5.2-r7 ACCEPT_KEYWORDS="x86 ~x86" AUTOCLEAN="yes" CFLAGS="-march=athlon-xp -mcpu=athlon-xp -O3 -pipe" CHOST="i686-pc-linux-gnu" COMPILER="" CONFIG_PROTECT="/etc /usr/X11R6/lib/X11/xkb /usr/kde/2/share/config /usr/kde/3/s hare/config /usr/share/config /usr/share/texmf/dvipdfm/config/ /usr/share/texmf/ dvips/config/ /usr/share/texmf/tex/generic/config/ /usr/share/texmf/tex/platex/c onfig/ /usr/share/texmf/xdvi/ /var/bind /var/qmail/alias /var/qmail/control /var /vpopmail/domains /var/vpopmail/etc" CONFIG_PROTECT_MASK="/etc/gconf /etc/terminfo /etc/env.d" CXXFLAGS="-march=athlon-xp -mcpu=athlon-xp -O3 -pipe" DISTDIR="/usr/gentoo-distfiles" FEATURES="autoaddcvs autoconfig buildpkg ccache confcache cvs digest distcc distlocks sandbox sfperms sign userpriv" GENTOO_MIRRORS="http://gentoo.ccccom.com http://gentoo.seren.com/gentoo" MAKEOPTS="-j1" PKGDIR="/usr/gentoo-packages" PORTAGE_TMPDIR="/var/tmp" PORTDIR="/usr/gentoo-cvs/gentoo-x86" PORTDIR_OVERLAY="/usr/local/portage" SYNC="" USE="3dnow aalib acl acpi alsa amd apache2 apm berkdb bitmap-fonts cdr cgi clearpasswd crypt cscope cups curl divx4linux f77 fam foomaticdb fortran gd gdbm geoip gif imagemagick imap innodb ipalias ipv6 java jikes jpeg junit libwww mad maildir mcal md5sum mikmod mmx mpeg multitarget mysql ncurses offensive pam pcap pdflib perl pic plotutils png pnp ppds python qmail readline samba scanner slp snmp socks5 spell sqlite sse ssl tetex tiff truetype type1 ungif usb userlocales v4l v4l2 x86 xml xml2 xvid zlib"
Created attachment 45580 [details] edtest.cpp Source code for testing binary.
Created attachment 45581 [details] dotest.sh Script file to run test case and prove the error.
Here is the patch (from LFS) that was applied by base-system to fix the vulnerability : ============================================================ --- ed-0.2/buf.c Sat Nov 19 04:37:59 1994 +++ ed-0.2-2/buf.c Tue May 28 18:38:23 2002 @@ -200,13 +200,13 @@ int open_sbuf () { - char *mktemp (); - int u; + int u, sfd; isbinary = newline_added = 0; u = umask(077); strcpy (sfn, "/tmp/ed.XXXXXX"); - if (mktemp (sfn) == NULL || (sfp = fopen (sfn, "w+")) == NULL) + sfd = mkstemp(sfn); + if ((sfd == -1) || (sfp = fopen (sfn, "w+")) == NULL) { fprintf (stderr, "%s: %s\n", sfn, strerror (errno)); sprintf (errmsg, "Cannot open temp file"); ============================================================= I fail to see where it introduces a regression, since apparently it just switched a mktemp for a mkstemp. Maybe someone more literate on those issues will find where it fails...
The error in your patch is that mkstemp returns a file descriptor for an OPEN file already (eg it runs fopen on the file). so: + sfd = mkstemp(sfn); + if ((sfd == -1) || (sfp = fopen (sfn, "w+")) == NULL) if mkstemp returns a good value, and then "sfp = fopen (sfn, "w+")" is run, opening the file again.
- if ((sfd == -1) || (sfp = fopen (sfn, "w+")) == NULL) + if (sfd == -1) so this patch will fix it ?
This wouldn't work, since not a file descriptor, but a FILE pointer is needed. However, I believe the patch from LFS is incorrect. Not fopen should be called, but fdopen on the descriptor: - if ((sfd == -1) || (sfp = fopen (sfn, "w+")) == NULL) + if ((sfd == -1) || (sfp = fdopen (sfd, "w+")) == NULL)
true ... robbat2, that patch looks much more sane, does that resolve this for you ?
Ok, your patch change there to use fdopen fixes the problem. I've put it in the tree as r5, but arches need to stabilize. The changed patch should probably be sent upstream.
marked stable ;)
