Bug 51632 - luit from x11/XFree86-4-clients is unusable
Summary: luit from x11/XFree86-4-clients is unusable
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Eric Anholt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-04-30 17:50 UTC by Guido Berhoerster
Modified: 2003-11-09 04:44 UTC (History)
0 users

See Also:


Attachments
x430-luit.diff (7.82 KB, patch)
2003-11-08 23:36 UTC, Eric Anholt
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Guido Berhoerster 2003-04-30 17:50:10 UTC
luit is a small utility which is part of the XFree86 4.3.0
distribution and is installed by the x11/XFree86-4-clients port.
luit adds locale and ISO 2022 support to Unicode terminals, most
notably xterm.
Unfortunately luit will leave the tty world writable if it is not
setuid root which renders it unusable on a default install of
x11/XFree86-4-clients. This problem is specific to systems without
SVR4 ptys like FreeBSD and documented on the manpage:

----snip----
On systems without SVR4 (``Unix-98'') ptys (notably BSD variants), run-
ning  luit  as an ordinary user will leave the tty world-writable; this
is a security hole, and luit will generate a warning (but still accept
to  run).   A  possible solution is to make luit suid root; luit should
drop privileges sufficiently early to make  this  safe. However,  the
startup code has not been exhaustively audited, and the author takes no
responsibility for any resulting security issues.
----snap----

Fix: 

This could be fixed by installing luit setuid root. If this is
considered too much of a security risk maybe a switch could be added
to the port so that people who need a localized xterm have an
option.
How-To-Repeat: 
For example try to get a xterm with German localization with

setenv LANG de_DE.ISO8859-15
setenv MM_CHARSET ISO-8859-15
xterm -lc -fa "Luxi Mono"

The resulting xterm prints a warning "Warning: could not change
ownership of tty -- pty is insecure!" and the corresponding tty is
of course world writable.
Comment 1 Tilman Keskinoz freebsd_committer 2003-04-30 19:06:23 UTC
Responsible Changed
From-To: freebsd-ports-bugs->anholt

Over to maintainer
Comment 2 ITO Tsuyoshi 2003-08-18 08:26:43 UTC
Note that the version of luit in XFree86 4.3.0 refuses to run when it
is setuid'ed root on FreeBSD.  This problem is solved in the version
in XFree86 CVS HEAD (Aug 17 20:39:58 2003 UTC).

See:
  http://www.mail-archive.com/devel@xfree86.org/msg01547.html
  http://www.mail-archive.com/devel@xfree86.org/msg01317.html
  http://cvsweb.xfree86.org/cvsweb/xc/programs/luit/sys.c#rev1.9

Just updating luit to XFree86 CVS HEAD solves the problem on my
FreeBSD 4.8-RELEASE-p* environment.  I extracted the portion of the
diffs which I think is minimally necessary.

In addition, the patch below adds InstallLuitSetUID variable to
x11/XFree86-4-clients port.

After applying the patch, do
$ cd /usr/ports/x11/XFree86-4-clients && make InstallLuitSetUID=yes install

Best regards,
Tsuyoshi

---   ITO Tsuyoshi  <tsuyoshi@is.s.u-tokyo.ac.jp>   ---
--- Dept. of Computer Science, University of Tokyo. ---

Index: x11/XFree86-4-clients/Makefile
===================================================================
RCS file: /home/ncvs/ports/x11/XFree86-4-clients/Makefile,v
retrieving revision 1.114
diff -u -r1.114 Makefile
--- x11/XFree86-4-clients/Makefile	12 Aug 2003 18:35:36 -0000	1.114
+++ x11/XFree86-4-clients/Makefile	18 Aug 2003 07:12:18 -0000
@@ -7,7 +7,7 @@
 
 PORTNAME=	clients
 PORTVERSION=	4.3.0
-PORTREVISION=	3
+PORTREVISION=	4
 CATEGORIES=	x11
 MASTER_SITES=	${MASTER_SITE_XFREE}
 MASTER_SITE_SUBDIR=	4.3.0
@@ -45,12 +45,19 @@
 # InstallXdmConfig      YES     install config files for xdm.
 # InstallXinitConfig    YES     install config files for xinit.
 # InstallAppDefFiles    YES	install resource files.
+# InstallLuitSetUID     (undef)	if defined, install bin/luit with setuid bit.
 # ----------------------------------------------------------------------------
 # DEFAULT means ports will use values which set by ${PORTSDIR}/devel/imake-4
 #
 InstallXdmConfig?=	DEFAULT
 InstallXinitConfig?=	DEFAULT
 InstallAppDefFiles?=	DEFAULT
+
+.if defined(InstallLuitSetUID)
+post-install:
+	${CHOWN} root:wheel ${PREFIX}/bin/luit
+	${CHMOD} 04711 ${PREFIX}/bin/luit
+.endif
 
 .include "${.CURDIR}/../../x11/XFree86-4-libraries/Makefile.inc"
 .include <bsd.port.pre.mk>
Index: x11/XFree86-4-libraries/files/patch-luit
===================================================================
RCS file: /home/ncvs/ports/x11/XFree86-4-libraries/files/patch-luit,v
retrieving revision 1.1
diff -u -r1.1 patch-luit
--- x11/XFree86-4-libraries/files/patch-luit	11 Mar 2003 23:38:07 -0000	1.1
+++ x11/XFree86-4-libraries/files/patch-luit	18 Aug 2003 07:12:18 -0000
@@ -10,7 +10,7 @@
  SRCS = luit.c iso2022.c charset.c parser.c sys.c other.c
  
 --- programs/luit/sys.c.orig	Mon Jan  7 12:38:30 2002
-+++ programs/luit/sys.c	Tue Mar 11 14:57:02 2003
++++ programs/luit/sys.c	Sun Aug 17 15:16:18 2003
 @@ -33,6 +33,7 @@
  #include <termios.h>
  #include <signal.h>
@@ -19,7 +19,18 @@
  
  #ifdef SVR4
  #define HAVE_POLL
-@@ -313,6 +314,7 @@
+@@ -68,6 +69,10 @@
+ #include <stropts.h>
+ #endif
+ 
++#if (defined(__unix__) || defined(unix)) && !defined(USG)
++#include <sys/param.h>
++#endif
++
+ #include "sys.h"
+ 
+ static int saved_tio_valid = 0;
+@@ -313,6 +318,7 @@
      int pty = -1;
      char *name1 = "pqrstuvwxyzPQRST", *name2 = "0123456789abcdef";
      char *p1, *p2;
@@ -27,7 +38,7 @@
  
  #ifdef HAVE_GRANTPT
      char *temp_line;
-@@ -355,27 +357,11 @@
+@@ -355,27 +361,11 @@
    bsd:
  #endif /* HAVE_GRANTPT */
  
@@ -59,3 +70,41 @@
      fix_pty_perms(line);
      *pty_return = pty;
      *line_return = line;
+@@ -429,7 +419,10 @@
+     return -1;
+ }
+ 
+-#ifdef _POSIX_SAVED_IDS
++/* Post-4.4 BSD systems have POSIX semantics (_POSIX_SAVED_IDS
++   or not, depending on the version).  4.3BSD and Minix do not have
++   saved IDs at all, so there's no issue. */
++#if (defined(BSD) && !defined(_POSIX_SAVED_IDS)) || defined(_MINIX)
+ int
+ droppriv()
+ {
+@@ -438,6 +431,25 @@
+     if(rc < 0)
+         return rc;
+     return setgid(getgid());
++}
++#elif defined(_POSIX_SAVED_IDS)
++int
++droppriv()
++{
++    int uid = getuid();
++    int euid = geteuid();
++    int gid = getgid();
++    int egid = getegid();
++    int rc;
++
++    if((uid != euid || gid != egid) && euid != 0) {
++        errno = ENOSYS;
++        return -1;
++    }
++    rc = setuid(uid);
++    if(rc < 0)
++        return rc;
++    return setgid(gid);
+ }
+ #else
+ int
Comment 3 Eric Anholt freebsd_committer 2003-11-08 23:36:56 UTC
On Mon, 2003-08-18 at 00:26, ITO Tsuyoshi wrote:
> Note that the version of luit in XFree86 4.3.0 refuses to run when it
> is setuid'ed root on FreeBSD.  This problem is solved in the version
> in XFree86 CVS HEAD (Aug 17 20:39:58 2003 UTC).
> 
> See:
>   http://www.mail-archive.com/devel@xfree86.org/msg01547.html
>   http://www.mail-archive.com/devel@xfree86.org/msg01317.html
>   http://cvsweb.xfree86.org/cvsweb/xc/programs/luit/sys.c#rev1.9
> 
> Just updating luit to XFree86 CVS HEAD solves the problem on my
> FreeBSD 4.8-RELEASE-p* environment.  I extracted the portion of the
> diffs which I think is minimally necessary.
> 
> In addition, the patch below adds InstallLuitSetUID variable to
> x11/XFree86-4-clients port.
> 
> After applying the patch, do
> $ cd /usr/ports/x11/XFree86-4-clients && make InstallLuitSetUID=yes install

It seems to me that if we're going to grab most of HEAD, we should just
patch the 4.3.0 luit up to CVS HEAD.  It sounds like the other patches
are useful, plus we can be sure no patches will be necessary when 4.4.0
comes out.

Attached is a patch that does so, along with installing luit setuid
unconditionally.  However, in trying to test it with the locale settings
from the originator, the xterm generally flashes on the screen and
exits, and only a few times has successfully come up for me, though it
was without the warning about insecurity.  Any idea what would be going
on?

-- 
Eric Anholt                                eta@lclark.edu          
http://people.freebsd.org/~anholt/         anholt@FreeBSD.org
Comment 4 ITO Tsuyoshi 2003-11-09 01:18:33 UTC
From: Eric Anholt <anholt@FreeBSD.org>
Date: Sat, 08 Nov 2003 15:36:56 -0800
> It seems to me that if we're going to grab most of HEAD, we should just
> patch the 4.3.0 luit up to CVS HEAD.  It sounds like the other patches
> are useful, plus we can be sure no patches will be necessary when 4.4.0
> comes out.
I agree.

> Attached is a patch that does so, along with installing luit setuid
> unconditionally.
I am afraid that to those who do not need luit, it may be a security
concern without any advantage.

>                   However, in trying to test it with the locale settings
> from the originator, the xterm generally flashes on the screen and
> exits, and only a few times has successfully come up for me, though it
> was without the warning about insecurity.  Any idea what would be going
> on?
No.  I tested the same settings from the originator and saw only
flushing you describe.  Just
  setenv LANG de_DE.ISO8859-15 && xterm -lc
  (without MM_CHARSET)
caused the same behavior, though I do not know what MM_CHARSET or '-lc
-fa "Luci Mono"' means.  I tested with the minimally-patched version,
not the HEAD version.

On the other hand, with LANG set to ja_JP.eucJP, 'xterm -lc' goes
successfully.  I wonder what the difference between de_DE.ISO8859-15
and ja_JP.eucJP is there.

Besides, I experience the same "just flush and exit" behavior once in
more than ten times when I execute "xterm" with LANG set to
ja_JP.eucJP.  I do not know what causes this, whether this is the same
problem as the de_DE.ISO8859-15 case, or whether this is FreeBSD
specific or not.  The changes to search a free pty harder (part of
luit/sys.c rev 1.8) is not related because the problem occurs even
when I execute the first xterm.

Tsuyoshi
Comment 5 Eric Anholt freebsd_committer 2003-11-09 04:44:37 UTC
State Changed
From-To: open->closed

Committed, thanks!