Bugzilla Bug 219750

mrxvt-0.5.3 is vulnerable to the same X11 Display issue as rxvt,

"The security issue is caused due to the program using ":0" as it's X11 display
if the DISPLAY environment variable is missing. This can be exploited to
execute arbitrary commands with the privileges of the user running rxvt via a
malicious X server."

rxvt bug #217819

Setting whiteboard and cc.

Adding Gautam Iyer, as he is an upstream for mrxvt and has account here.

Gautam: could you check out the patch in bug #217819 and provide new version of
mrxvt?

Created an attachment (id=151368) [details]
Patch fixing the security issue

Here's the patch (now in SVN).

This patch will have (almost) no effect unless the LOCAL_X_IS_UNIX flag is
defined in src/feature.h. When this flag is defined, mrxvt will force a "unix:"
to be prepended to (local) display strings passed to the X server.

If this flag is not defined, then mrxvt will use the -display option unchanged,
or pass NULL to XOpenDisplay(), letting the server get the display from the
environment. (If display is undefined, mrxvt will exit). This is what happens
with most X programs on my currently installed Gentoo laptop.

I'm a little reluctant to #define LOCAL_X_IS_UNIX by default. It might break
something for someone. Would it be possible for you to #define LOCAL_X_IS_UNIX
based on some USE flag? I don't know if "hardened" is appropriate, since that
doesn't necessarily mean that the X server will like "unix:0.0" displays. I
also notice that most other X programs don't care about this. (They either pass
NULL, or the display option verbatim to XOpenDisplay() ).

GI

(In reply to comment #3)
> Here's the patch (now in SVN).

Great :) Thank you for fast response.

> I'm a little reluctant to #define LOCAL_X_IS_UNIX by default. It might break
> something for someone. Would it be possible for you to #define LOCAL_X_IS_UNIX
> based on some USE flag? I don't know if "hardened" is appropriate, since that
> doesn't necessarily mean that the X server will like "unix:0.0" displays. I
> also notice that most other X programs don't care about this. (They either pass
> NULL, or the display option verbatim to XOpenDisplay() ).

I have left it undefined for now. That way mrxvt will rely on X server to take
care of DISPLAY string, so if user wants to shoot his foot he can do it.


Security: x11-terms/mrxvt-0.5.3-r2 added to the tree, target KEYWORDS="alpha
amd64 ~mips ppc x86", as 0.5.3-r2 has only the above patch in comparison with
0.5.2-r1 the stabilization should be an easy one.

Testing procedure:
 * open xterm
 * start mrxvt from xterm - should open on same display
 * unset DISPLAY in xterm
 * try to start mrxvt, it should fail with: "Error opening display (null)"

(In reply to comment #4)
>> I'm a little reluctant to #define LOCAL_X_IS_UNIX by default. It
>> might break something for someone. Would it be possible for you to
>> #define LOCAL_X_IS_UNIX based on some USE flag? I don't know if
>> "hardened" is appropriate, since that doesn't necessarily mean that
>> the X server will like "unix:0.0" displays. I also notice that most
>> other X programs don't care about this. (They either pass NULL, or
>> the display option verbatim to XOpenDisplay() ).
> 
> I have left it undefined for now. That way mrxvt will rely on X server to take
> care of DISPLAY string, so if user wants to shoot his foot he can do it.

Ok. I realize that the original reporter meant something slightly
different: If DISPLAY is unset (or unusable) then mrxvt used to connect
to :0.0 by default. Thus if you have a malicious X server, it can fool
the user into connecting to the wrong one.

It didn't seem much of an issue to me. X server's are usually suid root.
They don't need to fool anyone into connecting to do something
malicious...

Either way, this is not an issue now. 

> Security: x11-terms/mrxvt-0.5.3-r2 added to the tree, target KEYWORDS="alpha
> amd64 ~mips ppc x86", as 0.5.3-r2 has only the above patch in comparison with
> 0.5.2-r1 the stabilization should be an easy one.

Thanks for the updated ebuild,

GI

Arches, please test and mark stable (as noted in comment #4)
x11-terms/mrxvt-0.5.3-r2
Target keywords: alpha amd64 ~mips ppc x86
Already stabled: amd64

x86 stable

alpha stable

ppc stable

GLSA 200805-03