Package: x11-common
Version: 1:7.3+2

Hello,

the new X version seems to drop privileges a little too early when it
gets started with -xf86config:

| holbe@darkside:/home/holbe% X :1 -xf86config xorg-tv.conf -nolisten tcp
|
| Fatal server error:
| Cannot move old log file ("/var/log/Xorg.1.log" to "/var/log/Xorg.1.log.old"
|
| [1]    12282 exit 1     X :1 -xf86config xorg-tv.conf -nolisten tcp
| holbe@darkside:/home/holbe%

Leaving -xf86config away, everything works as expected:

| holbe@darkside:/home/holbe% X :1 -nolisten tcp
|
| X.Org X Server 1.4.0
| Release Date: 5 September 2007
| X Protocol Version 11, Revision 0
| Build Operating System: Linux Debian (xorg-server 2:1.4-2)
| ...

The previous version 1:7.2-5 did work well in both cases.


Thanks for your work & regards
   Mario

Mario 'BitKoenig' Holbe wrote:

If you look at the beginning of the log, you should see that it didn't
find the xorg-tv.conf file.

This is on purpose, for security reasons. Upstream X.org enables the use
of any file as a custom config file. But the server outputs the first
broken line in the parse error in the log. It makes it possible to any
user to read the first line of whichever file in the system by just
passing it to the X server through -config or -xf86config.

So -config and -xf86config are modified in Debian to only accepts custom
config files in /etc/X11. You didn't have the problem with 7.2-5 because
we only modified -config. Now we modify -xf86config too since the same
security issue exists there too.

Brice

package x11-common
reopen 442879
thanks

No, I don't. Neither a logfile is moved nor is one written. I don't see
anything.

But as you state yourself below, Debian doesn't. And even more - it does
not accept paths in the conf-file, that's why I have to give the plain
name:

holbe@darkside:/home/holbe% X :1 -xf86config /etc/X11/xorg-tv.conf
-nolisten tcp

Fatal server error:

Invalid argument for -xf86config
        For non-root users, the file specified with -xf86config must be
        a relative path and must not contain any ".." elements.
        Using default xorg.conf search path.

Well, they just don't.

All right, so maybe I should just rename the bug to "-config/-xf86config
don't work anymore for non-root users" - if you like this more.

However, the issue remains: I cannot start any X server with a -config
or -xf86config option because of the error I mentioned in my first mail.


Mario

<snip>
That's not exactly true.  Upstream Xorg allows -config/-xf86config for
non-root with files under /etc/X11/ (given with a relative path), while
the Debian wrapper drops privileges when these options are used, without
looking at their argument.
Maybe we should change this, but otoh I'm not sure all files under
/etc/X11/ should be allowed (/etc/X11/Xwrapper.config, e.g., is only
readable by root).

Cheers,
Julien

Btw... while thinking about it... I belive to remember that I once tried
to switch from -xf86config to -config when I switched from XFree to Xorg
but just left it because -config didn't work while -xf86config did.

Well, IMHO it makes sense to allow different conf-files for non-root
users (for me it does :)). I understand that those files should be
supplied or at least be allowed by root, i.e. need to be placed in
/etc/X11. Afaics, Xwrapper.config is the *only* file there non-readable
for users - for whatever reason, since I'm easily able to reconstruct it
by just reading /var/lib/dpkg/info/x11-common.postinst and
/var/cache/debconf/config.dat.


Mario

Here's the patch I used to get it working again.


diff -u -p /home/gno/.emacs.backup/\!home\!gno\!xorg-7.3\+10\!debian\!local\!xserver-wrapper.c\~ /home/gno/xorg-7.3\+10/debian/local/xserver-wrapper.c
--- /home/gno/.emacs.backup/!home!gno!xorg-7.3+10!debian!local!xserver-wrapper.c~                2007-12-03 20:52:57.000000000 -0800
+++ /home/gno/xorg-7.3+10/debian/local/xserver-wrapper.c                                         2008-03-14 23:23:10.000000000 -0700
@@ -324,10 +324,14 @@ main(int argc, char **argv)

     for (i = 1; i < argc; i++) {
       if (!strcmp(argv[i], "-config") || !strcmp(argv[i], "-xf86config")) {
-        if (setuid(getuid())) {
-          perror("X unable to drop setuid privileges for alternate config");
-          exit(1);
-        }
+        if (i+1 > argc
+            || strstr(argv[i+1],"Xwrapper.config")
+            || strstr(argv[i+1],"..")) {
+          if (setuid(getuid())) {
+            perror("X unable to drop setuid privileges for alternate config");
+            exit(1);
+          }
+        }
       } else if (strlen(argv[i]) > 256) {
         if (setuid(getuid())) {
           perror("X unable to drop setuid privileges for suspiciously long "

Hi,

Last night I ran into another system broken by this bug. That prompted
me to show the patch to the owner of the broken system. He pointed out
that I missed the case of a leading slash. There's an amended patch at
the end.

So, now left I'm wondering why there's been no response to this for
six months. Did procmail eat my previous messages? Do others see
problems with the patch? I'd really appreciate some feedback, and
failing that, I'd be happy to see the patch applied; I'm getting tired of
going down the debugging path of "hey, X just exited with some
nonsense error about being unable to move a log file, what the hell?
Oh yeah. X11-common. Let me grab my patched version. Drat, it can't be
installed with the version of Xorg on this system; guess I need to
build some new versions."

I don't mean to complain, I really do appreciate having someone taking
care of the X server on my machines, and I'm open to other approaches
to fixing this, just let me know.

Thanks for your consideration,
Greg



diff -u -p /home/gno/xorg-7.3\+10/debian/local/xserver-wrapper.c.mine /home/gno/xorg-7.3\+10/debian/local/xserver-wrapper.c
--- /home/gno/xorg-7.3+10/debian/local/xserver-wrapper.c.mine 2008-09-14 12:25:20.000000000 -0700
+++ /home/gno/xorg-7.3+10/debian/local/xserver-wrapper.c 2008-09-14 12:25:44.000000000 -0700
@@ -324,6 +324,10 @@ main(int argc, char **argv)

     for (i = 1; i < argc; i++) {
       if (!strcmp(argv[i], "-config") || !strcmp(argv[i], "-xf86config")) {
+        if (i+1 > argc  /* if there is no argument to config */
+            || (*(argv[i+1]) == '/')) /* or it starts with a slash */
+            || strstr(argv[i+1],"Xwrapper.config") /* or is our config file */
+            || strstr(argv[i+1],"..")) { /* or attempts to escape this dir */
           if (setuid(getuid())) {
             perror("X unable to drop setuid privileges for alternate
       config");
             exit(1);

Diff finished.  Sun Sep 14 12:26:02 2008

Hi!

I confirm that this bug exists on all 3 lenny machines here.

The -config and -xf86config options are completely broken for users, you can't
start an xserver when you pass them, even when using the default xorg.conf:

X :1 -xf86config xorg.conf

won't start when done as user, even when "everyone can start X" is activated
(dpkg-reconfigure x11-common)

I hope the fix can be applied for a future point release, because the bug
breaks these options for nearly everyone. You need them to start multiple
x-servers with different configurations to achieve:
- have a separate x-server on another graphic card
- run a second x-server for presentations, games,..
  with different resolution, mouse support, ...

The patch did not work, because there was a wrong number of parentheses after
applying. The patch which worked for me is attached.

By not starting as user, e.g. with sudo.

xserver-xorg is a very small package so it did not take so long to patch and
build it, thanks for making it so modular.

Markus