Debian Bug report logs - #514406
xautolock: Uses freed memory for starting the locker

Display info messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Uli <ToBeSpammed@web.de>

To: submit@bugs.debian.org

Subject: xautolock: Uses freed memory for starting the locker

Date: Sat, 07 Feb 2009 10:39:02 +0100

Package: xautolock
Version: 1:2.1-7
Severity: grave
Justification: user security hole
Tags: security

xautolock uses an already freed memory address for starting the locker.

valgrind says:

==6017== Syscall param execve(argv[i]) points to unaddressable byte(s)
==6017==    at 0x55E43A7: execve (in /lib/libc-2.7.so)
==6017==    by 0x55E479A: execl (in /lib/libc-2.7.so)
==6017==    by 0x404026: (within /usr/bin/xautolock)
==6017==    by 0x40427B: (within /usr/bin/xautolock)
==6017==    by 0x55641A5: (below main) (in /lib/libc-2.7.so)
==6017==  Address 0x62ddcf0 is 16 bytes inside a block of size 65 free'd
==6017==    at 0x4C2130F: free (vg_replace_malloc.c:323)
==6017==    by 0x52852AA: (within /usr/lib/libX11.so.6.2.0)
==6017==    by 0x5285314: (within /usr/lib/libX11.so.6.2.0)
==6017==    by 0x52853B2: XrmDestroyDatabase (in /usr/lib/libX11.so.6.2.0)
==6017==    by 0x40334C: (within /usr/bin/xautolock)
==6017==    by 0x4040DE: (within /usr/bin/xautolock)
==6017==    by 0x55641A5: (below main) (in /lib/libc-2.7.so)

I noticed this because whenever I let xautolock start from my .xsessionrc it
would fail to start my screen locker. Instead of this:
  swarp 840 525 ; xset dpms force off ; slock
it started something like this, according to strace (the corruption didn't
always look the same):
  swarp 840 525 ; xset dpms force off ; slo\377\377\300

Because xset turned off the screen, I didn't notice that slock wasn't started
and thus my screen wasn't locked, which is why I think this is a security issue.
Feel free to correct me. ;)

Greetings
Uli Schlachter

-- System Information:
Debian Release: 5.0
  APT prefers testing-proposed-updates
  APT policy: (500, 'testing-proposed-updates'), (500, 'testing')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.27.7wlan.2.0 (SMP w/2 CPU cores)
Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages xautolock depends on:
ii  libc6                         2.7-18     GNU C Library: Shared libraries
ii  libx11-6                      2:1.1.5-2  X11 client-side library
ii  libxext6                      2:1.0.4-1  X11 miscellaneous extension librar
ii  libxss1                       1:1.1.3-1  X11 Screen Saver extension library

Versions of packages xautolock recommends:
pn  xlockmore | xtrlock | xscreen <none>     (no description available)

xautolock suggests no packages.

-- no debconf information

-- 
"Do you know that books smell like nutmeg or some spice from a foreign land?"
                                                  -- Faber in Fahrenheit 451

Message #10 received at 514406@bugs.debian.org (full text, mbox, reply):

From: Uli <ToBeSpammed@web.de>

To: 514406@bugs.debian.org

Cc: control@bugs.debian.org, team@security.debian.org, patrick@linux-dev.org

Subject: A patch for bug 514406

Date: Sat, 07 Feb 2009 14:17:50 +0100

[Message part 1 (text/plain, inline)]

tags 514406 + patch
thanks

Hi,

I read some man pages, looked at the source code and came up with the attached
patched. I noticed that options.c has a function (addExecToCommand()) that
creates a copy of the locker command line if it doesn't contain any semicolons.
This means I only ran into this bug, because my locker contains semicolons!

I patched this function to just always unconditionally copy the string and to
never prepend the locker with 'exec', because stuff like 'a & b', 'a || b' etc
isn't handled. I doubt this will cause any problems, but feel free to come up
with a different fix.

With this patch applied, valgrind doesn't complain about wrong memory usages
anymore.

Uli

P.S.: Thanks to Patrick Matthäi for helping me with this bug mail. CC'd security
on his advice, blame him. ;)
-- 
"Do you know that books smell like nutmeg or some spice from a foreign land?"
                                                  -- Faber in Fahrenheit 451

[xauto.patch (text/x-patch, inline)]

diff -Nurp xautolock-2.1.orig/src/options.c xautolock-2.1/src/options.c
--- xautolock-2.1.orig/src/options.c	2002-01-15 16:37:33.000000000 +0100
+++ xautolock-2.1/src/options.c	2009-02-07 14:01:47.192402633 +0100
@@ -250,6 +254,8 @@ addExecToCommand (const char** command)
   *  actually consists of multiple ones, we need to look for `;'
   *  characters first. We can only err on the safe side here...
   */
+  /* FIXME: This would also need to handle other stuff like e.g. & */
+#if 0
   if (!strchr (*command, ';'))
   {
     char* tmp;
@@ -257,6 +263,14 @@ addExecToCommand (const char** command)
 		    "exec %s", *command);
     *command = tmp;
   }
+#else
+  /* Create a copy of the string or else XrmDestroyDatabase would free() that
+   * string from underneath us.
+   */
+  char* tmp = newArray (char, strlen (*command) + 1);
+  (void) strcpy (tmp, *command);
+  *command = tmp;
+#endif
 }
 #endif /* !VMS */

Message #17 received at 514406@bugs.debian.org (full text, mbox, reply):

From: Nico Golde <nion@debian.org>

To: Uli <ToBeSpammed@web.de>

Cc: 514406@bugs.debian.org, team@security.debian.org, patrick@linux-dev.org

Subject: Re: A patch for bug 514406

Date: Sat, 7 Feb 2009 17:36:48 +0100

[Message part 1 (text/plain, inline)]

Hi,
* Uli <ToBeSpammed@web.de> [2009-02-07 17:23]:
> I read some man pages, looked at the source code and came up with the attached
> patched. I noticed that options.c has a function (addExecToCommand()) that
> creates a copy of the locker command line if it doesn't contain any semicolons.
> This means I only ran into this bug, because my locker contains semicolons!
> 
> I patched this function to just always unconditionally copy the string and to
> never prepend the locker with 'exec', because stuff like 'a & b', 'a || b' etc
> isn't handled. I doubt this will cause any problems, but feel free to come up
> with a different fix.
> 
> With this patch applied, valgrind doesn't complain about wrong memory usages
> anymore.
> 
> Uli
> 
> P.S.: Thanks to Patrick Matthäi for helping me with this bug mail. CC'd security
> on his advice, blame him. ;)

I don't think this justifies a security update by the 
security team but please make sure this fix gets into lenny.

Cheers
Nico
-- 
Nico Golde - http://www.ngolde.de - nion@jabber.ccc.de - GPG: 0x73647CFF
For security reasons, all text in this mail is double-rot13 encrypted.

[Message part 2 (application/pgp-signature, inline)]

Message #22 received at 514406@bugs.debian.org (full text, mbox, reply):

From: Patrick Matthäi <patrick@linux-dev.org>

To: Uli <ToBeSpammed@web.de>, 514406@bugs.debian.org, team@security.debian.org, patrick@linux-dev.org, nion@debian.org

Subject: Re: A patch for bug 514406

Date: Sat, 07 Feb 2009 17:52:33 +0100

Nico Golde schrieb:
> 
> I don't think this justifies a security update by the 
> security team but please make sure this fix gets into lenny.

Hi nico,

what about current stable?

Message #27 received at 514406@bugs.debian.org (full text, mbox, reply):

From: Nico Golde <nion@debian.org>

To: Patrick Matthäi <patrick@linux-dev.org>

Cc: Uli <ToBeSpammed@web.de>, 514406@bugs.debian.org, team@security.debian.org

Subject: Re: A patch for bug 514406

Date: Sat, 7 Feb 2009 19:30:49 +0100

[Message part 1 (text/plain, inline)]

Hi,
* Patrick Matthäi <patrick@linux-dev.org> [2009-02-07 18:28]:
> Nico Golde schrieb:
> >I don't think this justifies a security update by the security team but please 
> >make sure this fix gets into lenny.
> 
> what about current stable?

That was what I was referring to. In my opinion the impact 
of this bug is pretty minor and looking at that we release 
in lenny in about a week I think this is a waste of time but 
I'm also sure if you just prepare a package and upload to 
stable security that someone will handle that.

Cheers
Nico
-- 
Nico Golde - http://www.ngolde.de - nion@jabber.ccc.de - GPG: 0x73647CFF
For security reasons, all text in this mail is double-rot13 encrypted.

[Message part 2 (application/pgp-signature, inline)]

Message #32 received at 514406@bugs.debian.org (full text, mbox, reply):

From: Vincent Fourmond <fourmond@debian.org>

To: 514406@bugs.debian.org

Cc: Uli <ToBeSpammed@web.de>

Subject: Intention to NMU bug 514406

Date: Tue, 10 Feb 2009 21:58:05 +0100

[Message part 1 (text/plain, inline)]

  Hello,

  I intend to NMU this bug as soon as possible. Please find attached the
debdiff for the NMU.

  Regards,

	Vincent

-- 
Vincent Fourmond, Debian Developer
http://vince-debian.blogspot.com/

If you put a large switch in some cave somewhere, with a sign on it
saying "End-of-the-World switch. PLEASE DO NOT TOUCH", the paint
wouldn't even have the time to dry.
 -- Terry Pratchet, Thief of Time

Vincent, listening to White Summer (live) (Led Zeppelin)

[xautolock_bug_514406.nmu.diff (text/plain, inline)]

diff -u xautolock-2.1/debian/changelog xautolock-2.1/debian/changelog
--- xautolock-2.1/debian/changelog
+++ xautolock-2.1/debian/changelog
@@ -1,3 +1,14 @@
+xautolock (1:2.1-7.1) unstable; urgency=high
+
+  * Non-maintainer upload.
+  * 10-fix-memory-corruption to fix a memory corruption problem leading to
+    a user security problem (closes: 514406). Thanks to 
+    Uli <ToBeSpammed@web.de> for spotting the problem and providing the fix.
+  * Urgency high since it is a user security hole that really should make
+    it into lenny.
+
+ -- Vincent Fourmond <fourmond@debian.org>  Tue, 10 Feb 2009 21:49:25 +0100
+
 xautolock (1:2.1-7) unstable; urgency=high
 
   * High-urgency upload for RC bugfix by Steve Langasek
diff -u xautolock-2.1/debian/patches/00list xautolock-2.1/debian/patches/00list
--- xautolock-2.1/debian/patches/00list
+++ xautolock-2.1/debian/patches/00list
@@ -1,0 +2 @@
+10-fix-memory-corruption
only in patch2:
unchanged:
--- xautolock-2.1.orig/debian/patches/10-fix-memory-corruption.dpatch
+++ xautolock-2.1/debian/patches/10-fix-memory-corruption.dpatch
@@ -0,0 +1,33 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## 10-fix-memory-corruption.dpatch by  <fourmond@debian.org>
+##
+## DP: Don't send a freed memory location to an exec system call !
+
+@DPATCH@
+diff -urNad xautolock-2.1~/src/options.c xautolock-2.1/src/options.c
+--- xautolock-2.1~/src/options.c	2002-01-15 16:37:33.000000000 +0100
++++ xautolock-2.1/src/options.c	2009-02-10 21:40:02.000000000 +0100
+@@ -250,6 +250,8 @@
+   *  actually consists of multiple ones, we need to look for `;'
+   *  characters first. We can only err on the safe side here...
+   */
++  /* FIXME: This would also need to handle other stuff like e.g. & */
++#if 0
+   if (!strchr (*command, ';'))
+   {
+     char* tmp;
+@@ -257,6 +259,14 @@
+ 		    "exec %s", *command);
+     *command = tmp;
+   }
++#else
++  /* Create a copy of the string or else XrmDestroyDatabase would free() that
++   * string from underneath us.
++   */
++  char* tmp = newArray (char, strlen (*command) + 1);
++  (void) strcpy (tmp, *command);
++  *command = tmp;
++#endif
+ }
+ #endif /* !VMS */
+ 

Message #37 received at 514406-close@bugs.debian.org (full text, mbox, reply):

From: Vincent Fourmond <fourmond@debian.org>

To: 514406-close@bugs.debian.org

Subject: Bug#514406: fixed in xautolock 1:2.1-7.1

Date: Tue, 10 Feb 2009 21:17:05 +0000

Source: xautolock
Source-Version: 1:2.1-7.1

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

xautolock_2.1-7.1.diff.gz
  to pool/main/x/xautolock/xautolock_2.1-7.1.diff.gz
xautolock_2.1-7.1.dsc
  to pool/main/x/xautolock/xautolock_2.1-7.1.dsc
xautolock_2.1-7.1_amd64.deb
  to pool/main/x/xautolock/xautolock_2.1-7.1_amd64.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 514406@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Vincent Fourmond <fourmond@debian.org> (supplier of updated xautolock 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 ftpmaster@debian.org)


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

Format: 1.8
Date: Tue, 10 Feb 2009 21:49:25 +0100
Source: xautolock
Binary: xautolock
Architecture: source amd64
Version: 1:2.1-7.1
Distribution: unstable
Urgency: high
Maintainer: Roland Stigge <stigge@antcom.de>
Changed-By: Vincent Fourmond <fourmond@debian.org>
Description: 
 xautolock  - Program launcher for idle X sessions
Closes: 514406
Changes: 
 xautolock (1:2.1-7.1) unstable; urgency=high
 .
   * Non-maintainer upload.
   * 10-fix-memory-corruption to fix a memory corruption problem leading to
     a user security problem (closes: 514406). Thanks to
     Uli <ToBeSpammed@web.de> for spotting the problem and providing the fix.
   * Urgency high since it is a user security hole that really should make
     it into lenny.
Checksums-Sha1: 
 cd6f3f057339838e50ec00e4c49ec5119152b8c0 1019 xautolock_2.1-7.1.dsc
 aea496412109a206a48c426c2c5575ff66acf363 6421 xautolock_2.1-7.1.diff.gz
 4145d19e59acb9d6b28e42f361441330ddd5d3d1 31636 xautolock_2.1-7.1_amd64.deb
Checksums-Sha256: 
 f5ad223bceb75e9c71ba6bcdfe54fbfa193a5c8643f83b45fbdcf11a8b1e184a 1019 xautolock_2.1-7.1.dsc
 04db85a93b39bee3a1bf46df986ba017586a5456dcfd54238b0133ab7e161961 6421 xautolock_2.1-7.1.diff.gz
 81d9e691ecf2aedaaba3437ef5982cac7a29bd205d5e47f1e09888a1345d2252 31636 xautolock_2.1-7.1_amd64.deb
Files: 
 41f4164f7f23556c8e213bd579170ecc 1019 x11 optional xautolock_2.1-7.1.dsc
 c1fefdfe1977a491757d4901e942c9e9 6421 x11 optional xautolock_2.1-7.1.diff.gz
 1f28ecc7da4f17f34f2cc73f4debc2b7 31636 x11 optional xautolock_2.1-7.1_amd64.deb

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

iEYEARECAAYFAkmR7MYACgkQx/UhwSKygspeAwCgsWdJ9i8L0w2HwZKzuvHL9pQZ
KX4An0SFy2ghJui3nDxMRCTY5JlDwzRt
=airw
-----END PGP SIGNATURE-----

Send a report that this bug log contains spam.