Bug#328365: temporary file race in texindex

2005-10-05 Thread Florian Weimer 
* Norbert Preining:

 +  fd = open (name, O_CREAT|O_EXCL|O_WRONLY, 0666);

0600?  0666 might lead to an information leak.

 @@ -1615,14 +1626,15 @@
  /* Return a newly-allocated string concatenating S1 and S2.  */

This comment is outdated after the patch.


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]

Bug#328365: temporary file race in texindex

2005-10-05 Thread Norbert Preining 
Karl?

Your cvs also shows 0666. I guess 0600 would be ok.

On Mit, 05 Okt 2005, Florian Weimer wrote:
 * Norbert Preining:
 
  +  fd = open (name, O_CREAT|O_EXCL|O_WRONLY, 0666);
 
 0600?  0666 might lead to an information leak.
 
  @@ -1615,14 +1626,15 @@
   /* Return a newly-allocated string concatenating S1 and S2.  */
 
 This comment is outdated after the patch.

Best wishes

Norbert

---
Dr. Norbert Preining preining AT logic DOT at Università di Siena
sip:[EMAIL PROTECTED] +43 (0) 59966-690018
gpg DSA: 0x09C5B094  fp: 14DF 2E6C 0307 BE6D AD76  A9C0 D2BF 4AA3 09C5 B094
---
BRECON
That part of the toenail which is designed to snag on nylon sheets.
--- Douglas Adams, The Meaning of Liff


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]

Bug#328365: temporary file race in texindex

2005-10-05 Thread Karl Berry 
0600?  0666 might lead to an information leak.

Thanks, changed.

This comment is outdated after the patch.

Right.  I fixed that comment and a couple other minor things when I applied.

Thanks,
Karl


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]

Bug#328365: temporary file race in texindex

2005-10-02 Thread Norbert Preining 
On Fre, 30 Sep 2005, Karl Berry wrote:
 I've adapted the OpenBSD stuff and created a patch. Maybe
 you want to look at it if this works.
 
 Thanks for doing this.
 
 In general, the OpenBSD code seems to be a couple versions back, as it
 has KR function definitions and omits a couple other changes I made
 (quite) a while ago.

In fact the patch seems to be against texinfo-4.8, the last released
version of texinfo.

One question: For the upcoming texinfo-4.8 package in Debian, can I use
the patch of Henry, Karl?

Best wishes

Norbert

---
Dr. Norbert Preining preining AT logic DOT at Università di Siena
sip:[EMAIL PROTECTED] +43 (0) 59966-690018
gpg DSA: 0x09C5B094  fp: 14DF 2E6C 0307 BE6D AD76  A9C0 D2BF 4AA3 09C5 B094
---
MEATHOP (n.)
One who sets off for the scene of an aircraft crash with a picnic
hamper.
--- Douglas Adams, The Meaning of Liff


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]

Bug#328365: temporary file race in texindex

2005-10-02 Thread Karl Berry 
In fact the patch seems to be against texinfo-4.8, the last released
version of texinfo.

Well, the patch applies, but that's not what I was saying.

One question: For the upcoming texinfo-4.8 package in Debian, can I use
the patch of Henry, Karl?

It's up to you, but I don't advise it.  It reverts many declarations to
KR form (i.e., a much older texindex.c).  Aside from that, it also
reverts at least one bug fix I made regarding initials (years ago).

Perhaps you or someone could work on just making a patch which fixes the
race condition without all the other extraneous (and undesirable) changes.
That would help me.

Thanks,
Karl


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]

Bug#328365: temporary file race in texindex

2005-10-02 Thread Norbert Preining 
On Son, 02 Okt 2005, Karl Berry wrote:
 It's up to you, but I don't advise it.  It reverts many declarations to
 KR form (i.e., a much older texindex.c).  Aside from that, it also
 reverts at least one bug fix I made regarding initials (years ago).
 
 Perhaps you or someone could work on just making a patch which fixes the
 race condition without all the other extraneous (and undesirable) changes.
 That would help me.

Can you please comment on my first try on this, attached. I removed all
the unneccessary stuff I found, and it still compiles. 

Please comment.

Best wishes

Norbert

---
Dr. Norbert Preining preining AT logic DOT at Università di Siena
sip:[EMAIL PROTECTED] +43 (0) 59966-690018
gpg DSA: 0x09C5B094  fp: 14DF 2E6C 0307 BE6D AD76  A9C0 D2BF 4AA3 09C5 B094
---
SCORRIER (n.)
A small hunting dog trained to snuffle amongst your private parts.
--- Douglas Adams, The Meaning of Liff
--- /src/debian-tex/pkg-texlive/texinfo/branches/upstream/4.8/util/texindex.c   
2004-04-11 19:56:47.0 +0200
+++ util/texindex.c 2005-10-02 23:47:05.0 +0200
@@ -99,6 +99,9 @@
 /* Directory to use for temporary files.  On Unix, it ends with a slash.  */
 char *tempdir;
 
+/* Start of filename to use for temporary files.  */
+char *tempbase;
+
 /* Number of last temporary file.  */
 int tempcount;
 
@@ -144,7 +147,7 @@
 void fatal (const char *format, const char *arg);
 void error (const char *format, const char *arg);
 void *xmalloc (), *xrealloc ();
-char *concat (char *s1, char *s2);
+char *concat (char *s1, char *s2, char *s3);
 void flush_tempfiles (int to_count);
 
 #define MAX_IN_CORE_SORT 50
@@ -190,6 +193,11 @@
 
   decode_command (argc, argv);
 
+  /* XXX mkstemp not appropriate, as we need to have somewhat predictable
+   * names. But race condition was fixed, see maketempname.
+   */
+  tempbase = mktemp (concat (txiXX, , ));
+
   /* Process input files completely, one by one.  */
 
   for (i = 0; i  num_infiles; i++)
@@ -220,7 +228,7 @@
 
   outfile = outfiles[i];
   if (!outfile)
-outfile = concat (infiles[i], s);
+outfile = concat (infiles[i], s, );
 
   need_initials = 0;
   first_initial = '\0';
@@ -318,7 +326,7 @@
   if (tempdir == NULL)
 tempdir = DEFAULT_TMPDIR;
   else
-tempdir = concat (tempdir, /);
+tempdir = concat (tempdir, /, );
 
   keep_tempfiles = 0;
 
@@ -384,26 +392,25 @@
 usage (1);
 }
 
-/* Return a name for temporary file COUNT. */
+/* Return a name for a temporary file. */
 
 static char *
 maketempname (int count)
 {
-  static char *tempbase = NULL;
   char tempsuffix[10];
+  char *name;
+  int fd;
 
-  if (!tempbase)
+  sprintf (tempsuffix, .%d, count);
+  name =  concat (tempdir, tempbase, tempsuffix);
+  fd = open (name, O_CREAT|O_EXCL|O_WRONLY, 0666);
+  if (fd == -1)
+return NULL;
+  else
 {
-  int fd;
-  tempbase = concat (tempdir, txidxXX);
-
-  fd = mkstemp (tempbase);
-  if (fd == -1)
-pfatal_with_name (tempbase);
+  close(fd);
+  return(name);
 }
-
-  sprintf (tempsuffix, .%d, count);
-  return concat (tempbase, tempsuffix);
 }
 
 
@@ -931,6 +938,8 @@
   for (i = 0; i  ntemps; i++)
 {
   char *newtemp = maketempname (++tempcount);
+  if (!newtemp)
+pfatal_with_name(temp file);
   sort_in_core (tempfiles[i], MAX_IN_CORE_SORT, newtemp);
   if (!keep_tempfiles)
 unlink (tempfiles[i]);
@@ -1401,6 +1410,8 @@
   if (i + 1 == ntemps)
 nf = nfiles - i * MAX_DIRECT_MERGE;
   tempfiles[i] = maketempname (++tempcount);
+  if (!tempfiles[i])
+   pfatal_with_name(temp file);
   value |= merge_direct (infiles[i * MAX_DIRECT_MERGE], nf, tempfiles[i]);
 }
 
@@ -1615,14 +1626,15 @@
 /* Return a newly-allocated string concatenating S1 and S2.  */
 
 char *
-concat (char *s1, char *s2)
+concat (char *s1, char *s2, char *s3)
 {
-  int len1 = strlen (s1), len2 = strlen (s2);
-  char *result = (char *) xmalloc (len1 + len2 + 1);
+  int len1 = strlen (s1), len2 = strlen (s2), len3 = strlen (s3);
+  char *result = (char *) xmalloc (len1 + len2 + len3 + 1);
 
   strcpy (result, s1);
   strcpy (result + len1, s2);
-  *(result + len1 + len2) = 0;
+  strcpy (result + len1 + len2, s3);
+  *(result + len1 + len2 + len3) = 0;
 
   return result;
 }

Bug#328365: temporary file race in texindex

2005-10-02 Thread Karl Berry 
Can you please comment on my first try on this, attached. 

That looks just fine.  I'll apply it later today or tomorrow.  Thanks
Norbert!


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]

Bug#328365: temporary file race in texindex

2005-09-30 Thread Henry Jensen 
Hello,

I've adapted the OpenBSD stuff and created a patch. Maybe
you want to look at it if this works.

Regards,
Henry



texindex-racecondition.patch
Description: Binary data

Bug#328365: temporary file race in texindex

2005-09-30 Thread Karl Berry 
I've adapted the OpenBSD stuff and created a patch. Maybe
you want to look at it if this works.

Thanks for doing this.

In general, the OpenBSD code seems to be a couple versions back, as it
has KR function definitions and omits a couple other changes I made
(quite) a while ago.

I wonder if you could start with the current texindex.c and make a patch
with only the mk*temp changes?  I could separate it out from your patch
given time, but it will happen that much faster if you wouldn't mind
going that extra step.

The race-condition fix itself looks fine to me, although I admit I am
not especially expert at such things.

Thanks again,
Karl


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]

Bug#328365: temporary file race in texindex (was: CAN number)

2005-09-28 Thread Frank Küster 
Martin Pitt [EMAIL PROTECTED] wrote:

 Hi!

 This has been assigned CAN-2005-3011, please mention this number in
 the changelog when you fix this to allow easy tracking.

The current version, 4.8, is as well vulnerable:

[EMAIL PROTECTED]:~$ diff -u 
src/packages_for_sponsoring/texinfo-4.{7,8}/util/texindex.c 
--- src/packages_for_sponsoring/texinfo-4.7/util/texindex.c 2004-03-18 
23:26:53.0 +0100
+++ src/packages_for_sponsoring/texinfo-4.8/util/texindex.c 2004-04-11 
19:56:47.0 +0200
@@ -1,5 +1,5 @@
 /* texindex -- sort TeX index dribble output into an actual index.
-   $Id: texindex.c,v 1.3 2004/03/18 22:26:53 karl Exp $
+   $Id: texindex.c,v 1.11 2004/04/11 17:56:47 karl Exp $
 
Copyright (C) 1987, 1991, 1992, 1996, 1997, 1998, 1999, 2000, 2001,
2002, 2003, 2004 Free Software Foundation, Inc.

I have no idea whether and how I can (request to) change the info in the CVE 
database.

Regards, Frank

P.S. Frank, since you seem to be working on the source code of 4.7,
maybe you want to join the discussion in #320413 about taking over the
package from Josip, who seems to be MIA.
-- 
Frank Küster
Inst. f. Biochemie der Univ. Zürich
Debian Developer

Bug#328365: temporary file race in texindex (was: CAN number)

2005-09-28 Thread Martin Pitt 
Hi Frank!

Frank Küster [2005-09-28 16:11 +0200]:
 The current version, 4.8, is as well vulnerable:
 
 [EMAIL PROTECTED]:~$ diff -u 
 src/packages_for_sponsoring/texinfo-4.{7,8}/util/texindex.c 
 --- src/packages_for_sponsoring/texinfo-4.7/util/texindex.c   2004-03-18 
 23:26:53.0 +0100
 +++ src/packages_for_sponsoring/texinfo-4.8/util/texindex.c   2004-04-11 
 19:56:47.0 +0200
 @@ -1,5 +1,5 @@
  /* texindex -- sort TeX index dribble output into an actual index.
 -   $Id: texindex.c,v 1.3 2004/03/18 22:26:53 karl Exp $
 +   $Id: texindex.c,v 1.11 2004/04/11 17:56:47 karl Exp $
  
 Copyright (C) 1987, 1991, 1992, 1996, 1997, 1998, 1999, 2000, 2001,
 2002, 2003, 2004 Free Software Foundation, Inc.

Lol, I hope this is not the only difference between the versions. :-)

 I have no idea whether and how I can (request to) change the info in
 the CVE database.

You can mail [EMAIL PROTECTED] and explain the issue, they will correct
it.

Thanks!

Martin

-- 
Martin Pitt  http://www.piware.de
Ubuntu Developer   http://www.ubuntulinux.org
Debian Developerhttp://www.debian.org


signature.asc
Description: Digital signature

Bug#328365: temporary file race in texindex (was: CAN number)

2005-09-28 Thread Frank Lichtenheld 
On Wed, Sep 28, 2005 at 04:11:48PM +0200, Frank Küster wrote:
 P.S. Frank, since you seem to be working on the source code of 4.7,
 maybe you want to join the discussion in #320413 about taking over the
 package from Josip, who seems to be MIA.

All my involvements with texinfo were either from the release team
perspective (as it is a basic package) or (in this case) out of
accident (one of my packages contains a local copy of the texinfo code
and I did a little security audit of it).

Please go ahead and highjack the package, it seems clearly warranted.
But I will not offer to co-maintain it, sorry...

Gruesse,
-- 
Frank Lichtenheld [EMAIL PROTECTED]
www: http://www.djpig.de/


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]

Bug#328365: temporary file race in texindex (was: CAN number)

2005-09-28 Thread Karl Berry 
 This has been assigned CAN-2005-3011, please mention this number in
 the changelog when you fix this to allow easy tracking.

Someone, please send me the actual bug report, and (hopefully) a fix.

Thanks,
karl


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]

Bug#328365: temporary file race in texindex (was: CAN number)

2005-09-28 Thread Frank Lichtenheld 
On Wed, Sep 28, 2005 at 10:58:51AM -0500, Karl Berry wrote:
  This has been assigned CAN-2005-3011, please mention this number in
  the changelog when you fix this to allow easy tracking.
 
 Someone, please send me the actual bug report, and (hopefully) a fix.

See http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=328365
I haven't provided a full patch in my original report, only a reference
to the OpenBSD patch (against an older version) which can be found at
http://www.openbsd.org/cgi-bin/cvsweb/src/gnu/usr.bin/texinfo/util/texindex.c.diff?r1=1.2r2=1.3
and could serve as a base.

Gruesse,
-- 
Frank Lichtenheld [EMAIL PROTECTED]
www: http://www.djpig.de/


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]

Bug#328365: temporary file race in texindex

2005-09-28 Thread Frank Küster 
[EMAIL PROTECTED] (Karl Berry) wrote:

  This has been assigned CAN-2005-3011, please mention this number in
  the changelog when you fix this to allow easy tracking.

 Someone, please send me the actual bug report, and (hopefully) a fix.

Karl, I forgot to ask you what happened to texindex.c between 4.7 and
4.8:  It increased its revision control version number from 1.3 to 1.11,
but there are no changes - have they all been reverted?

Regards, Frank
-- 
Frank Küster
Inst. f. Biochemie der Univ. Zürich
Debian Developer

Bug#328365: temporary file race in texindex

2005-09-28 Thread Frank Küster 
[EMAIL PROTECTED] (Karl Berry) wrote:

  This has been assigned CAN-2005-3011, please mention this number in
  the changelog when you fix this to allow easy tracking.

 Someone, please send me the actual bug report, and (hopefully) a fix.

Excuse me - any Debian bug report can be accessed via its bug number, so
this time it is

http://bugs.debian.org/328365

The text is:

,
| There is a race condition on creating temporary files in texindex.
| The following function generates the name of the temporary file:
| static char *
| maketempname (int count)
| {
|   static char *tempbase = NULL;
|   char tempsuffix[10];
| 
|   if (!tempbase)
| {
|   int fd;
|   tempbase = concat (tempdir, txidxXX);
| 
|   fd = mkstemp (tempbase);
|   if (fd == -1)
| pfatal_with_name (tempbase);
| }
| 
|   sprintf (tempsuffix, .%d, count);
|   return concat (tempbase, tempsuffix);
| }
| 
| which is used later as
| 
|  char *outname = maketempname (++tempcount);
|  FILE *ostream = fopen (outname, w);
| 
| Since the further filenames are deterministic after the first one
| is generated, this is easily exploitable.
| The use case is rather rare though since the temporary file are only
| generated if the file to sort is longer than 50.000 lines which is
| probably not too common.
`

Frank also commented on a potential patch:

,
| In OpenBSD this seems to have been fixed in 2000 (sic!), see this patch
| 
http://www.openbsd.org/cgi-bin/cvsweb/src/gnu/usr.bin/texinfo/util/texindex.c.diff?r1=1.2r2=1.3
| (which probably doesn't apply today cleanly anymore but could be adapted).
| It introduces a possibility for DoS but fixes the race...
| 
`

I don't see why texindex cannot simply use completely random filenames?
They are saved in an array and accessed as tempfiles[i], anyway.

Regards, Frank
-- 
Frank Küster
Inst. f. Biochemie der Univ. Zürich
Debian Developer

Bug#328365: temporary file race in texindex

2005-09-28 Thread Karl Berry 
It increased its revision control version number from 1.3 to 1.11,
but there are no changes - have they all been reverted?

There were no changes to texindex.c.  The $Id$ change isn't meaningful
-- it happened because of temporarily moving Texinfo to berlios (because
savannah was dead for months) and then moving it back.

| In OpenBSD this seems to have been fixed in 2000 (sic!), see this patch

Since the BSD folks (or Red Hat or anyone else but you, pretty much)
never bother to forward me any bugs or fixes they make, it's only by
random chance like this that I find out about them.  I rarely have time
to go seeking them out.

I don't see why texindex cannot simply use completely random filenames?

Sounds fine to me.  Any chance of sending me a clean patch?  The BSD
patch has so many conflicts that it is hard to tell what is really being
changed in this regard.

Thanks,
k


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]

Bug#328365: temporary file race in texindex

2005-09-14 Thread Frank Lichtenheld 
Package: texinfo
Version: 4.7-2.2
Severity: important
Tags: security

There is a race condition on creating temporary files in texindex.
The following function generates the name of the temporary file:
static char *
maketempname (int count)
{
  static char *tempbase = NULL;
  char tempsuffix[10];

  if (!tempbase)
{
  int fd;
  tempbase = concat (tempdir, txidxXX);

  fd = mkstemp (tempbase);
  if (fd == -1)
pfatal_with_name (tempbase);
}

  sprintf (tempsuffix, .%d, count);
  return concat (tempbase, tempsuffix);
}

which is used later as

 char *outname = maketempname (++tempcount);
 FILE *ostream = fopen (outname, w);

Since the further filenames are deterministic after the first one
is generated, this is easily exploitable.
The use case is rather rare though since the temporary file are only
generated if the file to sort is longer than 50.000 lines which is
probably not too common.

In OpenBSD this seems to have been fixed in 2000 (sic!), see this patch
http://www.openbsd.org/cgi-bin/cvsweb/src/gnu/usr.bin/texinfo/util/texindex.c.diff?r1=1.2r2=1.3
(which probably doesn't apply today cleanly anymore but could be adapted).
It introduces a possibility for DoS but fixes the race...

Gruesse,
Frank Lichtenheld

-- System Information:
Debian Release: testing/unstable
  APT prefers testing
  APT policy: (990, 'testing'), (500, 'unstable'), (500, 'stable'), (1, 
'experimental')
Architecture: i386 (i686)
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.8-2-k7
Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8)

Versions of packages texinfo depends on:
ii  libc6 2.3.5-6GNU C Library: Shared libraries an

texinfo recommends no packages.

-- no debconf information


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]