groffer uses temp files unsafely

Bug #9442 reported by Debian Bug Importer
8
Affects Status Importance Assigned to Milestone
groff (Debian)
Fix Released
Unknown
groff (Ubuntu)
Fix Released
High
Martin Pitt

Bug Description

Automatically imported from Debian bug report #278265 http://bugs.debian.org/278265

CVE References

Revision history for this message
Debian Bug Importer (debzilla) wrote :

Automatically imported from Debian bug report #278265 http://bugs.debian.org/278265

Revision history for this message
Debian Bug Importer (debzilla) wrote :

Message-ID: <email address hidden>
Date: Mon, 25 Oct 2004 16:13:54 -0400
From: Joey Hess <email address hidden>
To: Debian Bug Tracking System <email address hidden>
Subject: groffer uses temp files unsafely

--7JfCtLOvnd9MIVvH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Package: groff
Version: 1.18.1.1-1
Severity: serious
Tags: security

CAN-2004-0969 reported that groffer used temporary files in an
explitable manner. This version of groff seems to be vulnerable. A patch
is here:
http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=3D136313

-- System Information:
Debian Release: 3.1
  APT prefers unstable
  APT policy: (500, 'unstable')
Architecture: i386 (i686)
Kernel: Linux 2.4.27
Locale: LANG=3Den_US, LC_CTYPE=3Den_US (charmap=3DISO-8859-1)

Versions of packages groff depends on:
ii groff-base 1.18.1.1-1 GNU troff text-formatting syst=
em (
ii libc6 2.3.2.ds1-18 GNU C Library: Shared librarie=
s an
ii libgcc1 1:3.4.2-3 GCC support library
ii libice6 4.3.0.dfsg.1-8 Inter-Client Exchange library
ii libsm6 4.3.0.dfsg.1-8 X Window System Session Manage=
ment
ii libstdc++5 1:3.3.5-2 The GNU Standard C++ Library v3
ii libx11-6 4.3.0.dfsg.1-8 X Window System protocol clien=
t li
ii libxaw7 4.3.0.dfsg.1-8 X Athena widget set library
ii libxext6 4.3.0.dfsg.1-8 X Window System miscellaneous =
exte
ii libxmu6 4.3.0.dfsg.1-8 X Window System miscellaneous =
util
ii libxpm4 4.3.0.dfsg.1-8 X pixmap library
ii libxt6 4.3.0.dfsg.1-8 X Toolkit Intrinsics
ii xlibs 4.3.0.dfsg.1-8 X Window System client librari=
es m

-- no debconf information

--=20
see shy jo

--7JfCtLOvnd9MIVvH
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)

iD8DBQFBfV6Cd8HHehbQuO8RAk/HAKCrohPx5Q4X6qnYNzpj5IgC82IBhwCgsmK0
RcER4xcukwTQr6oNbixePTc=
=JusO
-----END PGP SIGNATURE-----

--7JfCtLOvnd9MIVvH--

Revision history for this message
In , Colin Watson (cjwatson) wrote : severity of 278265 is grave

severity 278265 grave

Revision history for this message
In , Colin Watson (cjwatson) wrote : Re: Bug#278265: groffer uses temp files unsafely
Download full text (3.2 KiB)

On Mon, Oct 25, 2004 at 04:13:54PM -0400, Joey Hess wrote:
> Package: groff
> Version: 1.18.1.1-1
> Severity: serious
> Tags: security
>
> CAN-2004-0969 reported that groffer used temporary files in an
> explitable manner. This version of groff seems to be vulnerable. A patch
> is here:
> http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=136313

I've backported this patch as follows:

--- groff-1.18.1.1.orig/debian/changelog
+++ groff-1.18.1.1/debian/changelog
@@ -1,3 +1,10 @@
+groff (1.18.1.1-2) unstable; urgency=high
+
+ * [SECURITY] Fix a race condition in groffer leading to a temporary file
+ handling vulnerability (closes: #278265).
+
+ -- Colin Watson <email address hidden> Tue, 26 Oct 2004 23:52:13 +0100
+
 groff (1.18.1.1-1) unstable; urgency=low

   * The "Death! Ride, ride to ruin and the world's ending!" release.
--- groff-1.18.1.1.orig/contrib/groffer/groffer.sh
+++ groff-1.18.1.1/contrib/groffer/groffer.sh
@@ -3228,17 +3228,12 @@
   do
     if is_not_empty "$d"; then
       if obj d is_dir && obj d is_writable; then
- _TMP_DIR="${d}/${_PROGRAM_NAME}${_PROCESS_ID}";
- if obj _TMP_DIR is_dir; then
- rm -f "${_TMP_DIR}"/*;
+ _TMP_DIR="$(mktemp -d "${d}/${_PROGRAM_NAME}.XXXXXX")"
+ if test $? = 0; then
           break;
         else
- mkdir "${_TMP_DIR}";
- if obj _TMP_DIR is_not_dir; then
- _TMP_DIR='';
- continue;
- fi;
- break;
+ _TMP_DIR='';
+ continue;
    fi;
       fi;
       if obj _TMP_DIR is_not_writable; then

Werner, Bernd, I'm not sure if you've been informed of this, but the
current groff CVS still seems to be vulnerable. The referenced Trustix
advisory doesn't go into detail, but there are two problems that I see:

  * groffer accepts a temporary directory that already exists and simply
    removes its contents. This is very unwise, especially since it does
    not check the permissions of that directory to make sure that nobody
    else has write access to it. The usual secure approach is only to
    accept a temporary directory that you have just created, so that you
    can be sure of its permissions. Trying to check is too fragile, and
    unnecessary.

  * groffer does not check the exit code of mkdir. It should do so,
    otherwise even if the above hole is plugged an attacker can create a
    world-writable directory between the is_dir check and the mkdir;
    since process IDs are predictable on most systems, this race
    condition is easy to win.

Following the patch in Red Hat's Bugzilla, I've simply used 'mktemp -d'
to fix this, because I know that's correct; however, it isn't portable,
so I suspect the patch above would not be accepted into groffer
upstream. Removing the code that accepts an existing temporary directory
and removes its contents, and then checking the exit code of mkdir,
ought to be sufficient; but I haven't tried this, and somebody should
audit it.

Even checking mkdir will still allow a denial-of-service attack, so this
would seem like a very good application for a helper written in C or C++
if you're unwilling to use mktemp when it's available.

Thanks,

--
Colin Watson ...

Read more...

Revision history for this message
In , Colin Watson (cjwatson) wrote : Bug#278265: fixed in groff 1.18.1.1-2

Source: groff
Source-Version: 1.18.1.1-2

We believe that the bug you reported is fixed in the latest version of
groff, which is due to be installed in the Debian FTP archive:

groff-base_1.18.1.1-2_powerpc.deb
  to pool/main/g/groff/groff-base_1.18.1.1-2_powerpc.deb
groff_1.18.1.1-2.diff.gz
  to pool/main/g/groff/groff_1.18.1.1-2.diff.gz
groff_1.18.1.1-2.dsc
  to pool/main/g/groff/groff_1.18.1.1-2.dsc
groff_1.18.1.1-2_powerpc.deb
  to pool/main/g/groff/groff_1.18.1.1-2_powerpc.deb

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed. If you
have further comments please address them to <email address hidden>,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Colin Watson <email address hidden> (supplier of updated groff package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing <email address hidden>)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Format: 1.7
Date: Tue, 26 Oct 2004 23:52:13 +0100
Source: groff
Binary: groff-base groff
Architecture: source powerpc
Version: 1.18.1.1-2
Distribution: unstable
Urgency: high
Maintainer: Colin Watson <email address hidden>
Changed-By: Colin Watson <email address hidden>
Description:
 groff - GNU troff text-formatting system
 groff-base - GNU troff text-formatting system (base system components)
Closes: 278265
Changes:
 groff (1.18.1.1-2) unstable; urgency=high
 .
   * [SECURITY] Fix a race condition in groffer leading to a temporary file
     handling vulnerability (closes: #278265).
Files:
 97346b285e2c0e85b0b0a6b3a9aa9aaa 761 text important groff_1.18.1.1-2.dsc
 f74f73e08483058021b665b3265f3096 118323 text important groff_1.18.1.1-2.diff.gz
 00e741276b666dd715282fe5d66364f5 860450 text important groff-base_1.18.1.1-2_powerpc.deb
 5c19e041ba3150726c8de67d0843de02 1885186 text optional groff_1.18.1.1-2_powerpc.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)
Comment: Colin Watson <email address hidden> -- Debian developer

iD8DBQFBftfk9t0zAhD6TNERArnBAJ9G4oykF36VAlAo80ukMtuyk2YT8ACeMUCe
5Tvw3ReQ8+MlWckOhMsvz1o=
=ATnd
-----END PGP SIGNATURE-----

Revision history for this message
In , Colin Watson (cjwatson) wrote : security => reopen until fixed in sarge

reopen 278265
tags 278265 sarge
thanks

--
Colin Watson [<email address hidden>]

Revision history for this message
Debian Bug Importer (debzilla) wrote :

Message-Id: <email address hidden>
Date: Tue, 26 Oct 2004 23:47:13 +0100
From: Colin Watson <email address hidden>
To: <email address hidden>
Subject: severity of 278265 is grave

severity 278265 grave

Revision history for this message
Debian Bug Importer (debzilla) wrote :
Download full text (3.5 KiB)

Message-ID: <email address hidden>
Date: Wed, 27 Oct 2004 00:30:33 +0100
From: Colin Watson <email address hidden>
To: Joey Hess <email address hidden>, <email address hidden>
Cc: Werner Lemberg <email address hidden>, Bernd Warken <email address hidden>,
 Martin Pitt <email address hidden>
Subject: Re: Bug#278265: groffer uses temp files unsafely

On Mon, Oct 25, 2004 at 04:13:54PM -0400, Joey Hess wrote:
> Package: groff
> Version: 1.18.1.1-1
> Severity: serious
> Tags: security
>
> CAN-2004-0969 reported that groffer used temporary files in an
> explitable manner. This version of groff seems to be vulnerable. A patch
> is here:
> http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=136313

I've backported this patch as follows:

--- groff-1.18.1.1.orig/debian/changelog
+++ groff-1.18.1.1/debian/changelog
@@ -1,3 +1,10 @@
+groff (1.18.1.1-2) unstable; urgency=high
+
+ * [SECURITY] Fix a race condition in groffer leading to a temporary file
+ handling vulnerability (closes: #278265).
+
+ -- Colin Watson <email address hidden> Tue, 26 Oct 2004 23:52:13 +0100
+
 groff (1.18.1.1-1) unstable; urgency=low

   * The "Death! Ride, ride to ruin and the world's ending!" release.
--- groff-1.18.1.1.orig/contrib/groffer/groffer.sh
+++ groff-1.18.1.1/contrib/groffer/groffer.sh
@@ -3228,17 +3228,12 @@
   do
     if is_not_empty "$d"; then
       if obj d is_dir && obj d is_writable; then
- _TMP_DIR="${d}/${_PROGRAM_NAME}${_PROCESS_ID}";
- if obj _TMP_DIR is_dir; then
- rm -f "${_TMP_DIR}"/*;
+ _TMP_DIR="$(mktemp -d "${d}/${_PROGRAM_NAME}.XXXXXX")"
+ if test $? = 0; then
           break;
         else
- mkdir "${_TMP_DIR}";
- if obj _TMP_DIR is_not_dir; then
- _TMP_DIR='';
- continue;
- fi;
- break;
+ _TMP_DIR='';
+ continue;
    fi;
       fi;
       if obj _TMP_DIR is_not_writable; then

Werner, Bernd, I'm not sure if you've been informed of this, but the
current groff CVS still seems to be vulnerable. The referenced Trustix
advisory doesn't go into detail, but there are two problems that I see:

  * groffer accepts a temporary directory that already exists and simply
    removes its contents. This is very unwise, especially since it does
    not check the permissions of that directory to make sure that nobody
    else has write access to it. The usual secure approach is only to
    accept a temporary directory that you have just created, so that you
    can be sure of its permissions. Trying to check is too fragile, and
    unnecessary.

  * groffer does not check the exit code of mkdir. It should do so,
    otherwise even if the above hole is plugged an attacker can create a
    world-writable directory between the is_dir check and the mkdir;
    since process IDs are predictable on most systems, this race
    condition is easy to win.

Following the patch in Red Hat's Bugzilla, I've simply used 'mktemp -d'
to fix this, because I know that's correct; however, it isn't portable,
so I suspect the patch above would not be accepted into groffer
upstream. Removing the code that accepts an existing temporary directory
and removes its contents, and...

Read more...

Revision history for this message
Debian Bug Importer (debzilla) wrote :

Message-Id: <email address hidden>
Date: Tue, 26 Oct 2004 19:32:10 -0400
From: Colin Watson <email address hidden>
To: <email address hidden>
Subject: Bug#278265: fixed in groff 1.18.1.1-2

Source: groff
Source-Version: 1.18.1.1-2

We believe that the bug you reported is fixed in the latest version of
groff, which is due to be installed in the Debian FTP archive:

groff-base_1.18.1.1-2_powerpc.deb
  to pool/main/g/groff/groff-base_1.18.1.1-2_powerpc.deb
groff_1.18.1.1-2.diff.gz
  to pool/main/g/groff/groff_1.18.1.1-2.diff.gz
groff_1.18.1.1-2.dsc
  to pool/main/g/groff/groff_1.18.1.1-2.dsc
groff_1.18.1.1-2_powerpc.deb
  to pool/main/g/groff/groff_1.18.1.1-2_powerpc.deb

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed. If you
have further comments please address them to <email address hidden>,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Colin Watson <email address hidden> (supplier of updated groff package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing <email address hidden>)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Format: 1.7
Date: Tue, 26 Oct 2004 23:52:13 +0100
Source: groff
Binary: groff-base groff
Architecture: source powerpc
Version: 1.18.1.1-2
Distribution: unstable
Urgency: high
Maintainer: Colin Watson <email address hidden>
Changed-By: Colin Watson <email address hidden>
Description:
 groff - GNU troff text-formatting system
 groff-base - GNU troff text-formatting system (base system components)
Closes: 278265
Changes:
 groff (1.18.1.1-2) unstable; urgency=high
 .
   * [SECURITY] Fix a race condition in groffer leading to a temporary file
     handling vulnerability (closes: #278265).
Files:
 97346b285e2c0e85b0b0a6b3a9aa9aaa 761 text important groff_1.18.1.1-2.dsc
 f74f73e08483058021b665b3265f3096 118323 text important groff_1.18.1.1-2.diff.gz
 00e741276b666dd715282fe5d66364f5 860450 text important groff-base_1.18.1.1-2_powerpc.deb
 5c19e041ba3150726c8de67d0843de02 1885186 text optional groff_1.18.1.1-2_powerpc.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)
Comment: Colin Watson <email address hidden> -- Debian developer

iD8DBQFBftfk9t0zAhD6TNERArnBAJ9G4oykF36VAlAo80ukMtuyk2YT8ACeMUCe
5Tvw3ReQ8+MlWckOhMsvz1o=
=ATnd
-----END PGP SIGNATURE-----

Revision history for this message
Debian Bug Importer (debzilla) wrote :

Message-ID: <email address hidden>
Date: Wed, 27 Oct 2004 01:01:20 +0100
From: Colin Watson <email address hidden>
To: <email address hidden>
Subject: security => reopen until fixed in sarge

reopen 278265
tags 278265 sarge
thanks

--
Colin Watson [<email address hidden>]

Revision history for this message
Matt Zimmerman (mdz) wrote :

Fixed via import from Debian:

     groff | 1.18.1.1-2 | http://archive.ubuntu.com hoary/main Sources

Revision history for this message
Colin Watson (cjwatson) wrote :

Doesn't this need to be fixed in warty too?

Revision history for this message
Martin Pitt (pitti) wrote :

Created an attachment (id=664)
interdiff for security update 1.18.1.1-1ubuntu0.1

same patch as for Hoary's version -2, but with more verbose changelog.

Revision history for this message
Martin Pitt (pitti) wrote :

Uploaded into Warty security queue, waiting to be ambered.

Revision history for this message
Martin Pitt (pitti) wrote :

Fixed in Warty:
 groff (1.18.1.1-1ubuntu0.1) warty-security; urgency=high
 .
   * SECURITY UPDATE: fix insecure temporary file creation
   * contrib/groffer/groffer.sh: use mktemp -d to securely create temporary
     directory; originally, a process id based approach was used (predictable,
     vulnerable to race conditions) and the directory was allowed to exist
     previously (vulnerable to overwrite user files)
   * References:
     CAN-2004-0969
     http://bugs.debian.org/278265

Already fixed in Hoary by syncing revision -2.

Revision history for this message
In , Werner Lemberg (wl) wrote : Re: Bug#278265: groffer uses temp files unsafely

> > Package: groff
> > Version: 1.18.1.1-1
> > Severity: serious
> > Tags: security
> >
> > CAN-2004-0969 reported that groffer used temporary files in an
> > explitable manner. This version of groff seems to be vulnerable. A
> > patch is here:
> > http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=136313

Bernd has fixed this differently; his solution is now in the CVS (see
http://savannah.gnu.org/cvs/?group=groff). Please test! If you can
verify that everything is OK I'll release a new groff version.

    Werner

Revision history for this message
Debian Bug Importer (debzilla) wrote :

Message-Id: <email address hidden>
Date: Tue, 16 Nov 2004 08:36:50 +0100 (CET)
From: Werner LEMBERG <email address hidden>
To: <email address hidden>
Cc: <email address hidden>, <email address hidden>, <email address hidden>, <email address hidden>
Subject: Re: Bug#278265: groffer uses temp files unsafely

> > Package: groff
> > Version: 1.18.1.1-1
> > Severity: serious
> > Tags: security
> >
> > CAN-2004-0969 reported that groffer used temporary files in an
> > explitable manner. This version of groff seems to be vulnerable. A
> > patch is here:
> > http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=136313

Bernd has fixed this differently; his solution is now in the CVS (see
http://savannah.gnu.org/cvs/?group=groff). Please test! If you can
verify that everything is OK I'll release a new groff version.

    Werner

Revision history for this message
In , Colin Watson (cjwatson) wrote : Re: Processed: security => reopen until fixed in sarge

On Wed, Oct 27, 2004 at 12:18:13AM -0700, Debian Bug Tracking System wrote:
> Processing commands for <email address hidden>:
>
> > reopen 278265
> Bug#278265: groffer uses temp files unsafely
> Bug reopened, originator not changed.
>
> > tags 278265 sarge
> Bug#278265: groffer uses temp files unsafely
> Tags were: security
> Tags added: sarge

The fixed version has been promoted to sarge.

--
Colin Watson [<email address hidden>]

Revision history for this message
Debian Bug Importer (debzilla) wrote :

Message-ID: <email address hidden>
Date: Wed, 17 Nov 2004 18:35:53 +0000
From: Colin Watson <email address hidden>
To: <email address hidden>
Subject: Re: Processed: security => reopen until fixed in sarge

On Wed, Oct 27, 2004 at 12:18:13AM -0700, Debian Bug Tracking System wrote:
> Processing commands for <email address hidden>:
>
> > reopen 278265
> Bug#278265: groffer uses temp files unsafely
> Bug reopened, originator not changed.
>
> > tags 278265 sarge
> Bug#278265: groffer uses temp files unsafely
> Tags were: security
> Tags added: sarge

The fixed version has been promoted to sarge.

--
Colin Watson [<email address hidden>]

Changed in groff:
status: Unknown → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.