X2Go Bug report logs - #333
users can inject data into X2Go Client using .bashrc

Toggle useless messages

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

From: "Dan Halbert" <halbert@halwitz.org>

To: submit@bugs.x2go.org

Subject: x2go client crashes if .bashrc prints anything

Date: Sat, 19 Oct 2013 12:22:43 -0400 (EDT)

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

Package: x2goclient
Version: 4.0.0.3
 
If I put an
echo "testing"   # exact text doesn't matter
 
at the top of my .bashrc, then the x2goclient crashes immediately when trying to start a session.
 
(The crash does not occur if I put a similar statement in .bash_login.)
 
I have reproduced this on the Windows client; I believe a colleague saw it on both the Windows and Linux clients.
 
The x2go server being used is  4.0.1.6-0~712~precise1.

[Message part 2 (text/html, inline)]

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

From: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>

To: Dan Halbert <halbert@halwitz.org>, 327@bugs.x2go.org

Cc: control@bugs.x2go.org

Subject: Re: [X2Go-Dev] Bug#327: x2go client crashes if .bashrc prints anything

Date: Tue, 29 Oct 2013 08:36:28 +0000

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

tag #327 confirmed
thanks

Hi Dan,

On  Sa 19 Okt 2013 18:22:43 CEST, Dan Halbert wrote:

> If I put an
> echo "testing"   # exact text doesn't matter

I presume, this on the server.

> at the top of my .bashrc, then the x2goclient crashes immediately  
> when trying to start a session.
>
> (The crash does not occur if I put a similar statement in .bash_login.)
>
> I have reproduced this on the Windows client; I believe a colleague  
> saw it on both the Windows and Linux clients.
>
> The x2go server being used is  4.0.1.6-0~712~precise1.

I can confirm that the issue exists with latest X2Go Client.

I could confirm this issue on Debian wheezy or Ubuntu precise as X2Go  
Server. On Ubuntu lucid, the problem does not occur.

Mike
-- 

DAS-NETZWERKTEAM
mike gabriel, herweg 7, 24357 fleckeby
fon: +49 (1520) 1976 148

GnuPG Key ID 0x25771B31
mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de

freeBusy:
https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xfb

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

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

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

From: Dan Halbert <halbert@halwitz.org>

To: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>, 327@bugs.x2go.org

Cc: control@bugs.x2go.org

Subject: Re: [X2Go-Dev] Bug#327: x2go client crashes if .bashrc prints anything

Date: Tue, 29 Oct 2013 07:55:05 -0400

On 10/29/2013 4:36 AM, Mike Gabriel wrote:
> If I put an
>> echo "testing"   # exact text doesn't matter
>
> I presume, this on the server.
Right, this is on the server. With the Windows client there is no 
.bashrc anyway. I confirmed with my colleague that he saw this on both 
the Windows and Ubuntu Precise clients.

Which windowing system chosen on the server does not seem to matter 
either. I saw it with UNITY and with just "Terminal".

> I could confirm this issue on Debian wheezy or Ubuntu precise as X2Go 
> Server. On Ubuntu lucid, the problem does not occur.
That's interesting. The reason for putting in the echo's was to debug a 
completely unrelated problem about which shell init got run when we were 
running some batch jobs. I had instrumented the init files before 
without difficulty. Thanks for looking at this.

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

From: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>

To: Dan Halbert <halbert@halwitz.org>

Cc: 327@bugs.x2go.org, control@bugs.x2go.org

Subject: Re: [X2Go-Dev] Bug#327: x2go client crashes if .bashrc prints anything

Date: Tue, 29 Oct 2013 12:23:16 +0000

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

clone #327 -1
tag #327 wontfix
retitle -1 users can inject data into X2Go Client using .bashrc
severity -1 grave

Hi Dan,

On  Di 29 Okt 2013 12:55:05 CET, Dan Halbert wrote:

> On 10/29/2013 4:36 AM, Mike Gabriel wrote:
>> If I put an
>>> echo "testing"   # exact text doesn't matter
>>
>> I presume, this on the server.
> Right, this is on the server. With the Windows client there is no  
> .bashrc anyway. I confirmed with my colleague that he saw this on  
> both the Windows and Ubuntu Precise clients.
>
> Which windowing system chosen on the server does not seem to matter  
> either. I saw it with UNITY and with just "Terminal".
>
>> I could confirm this issue on Debian wheezy or Ubuntu precise as  
>> X2Go Server. On Ubuntu lucid, the problem does not occur.
> That's interesting. The reason for putting in the echo's was to  
> debug a completely unrelated problem about which shell init got run  
> when we were running some batch jobs. I had instrumented the init  
> files before without difficulty. Thanks for looking at this.

I have looked at this in depth this morning. Indeed an echoing .bashrc  
file breaks X2Go. But it also breaks everything else around SSH, esp.  
scp [1, 2].

The first link [1] also provides a solution that I want to quote here:

""" (file: ~/.bashrc)
[... normal .bashrc stuff ...]

if [[ $- =~ "i" ]]; then
   echo "SPEAK OUT LOUD!!!"
fi
"""

The i-flag in $- checks if the shell is interactive or not. With X2Go,  
this flag will not get set.

Greets,
Mike

[1]  
http://stackoverflow.com/questions/12440287/scp-doesnt-work-when-echo-in-bashrc
[2] https://bugzilla.redhat.com/show_bug.cgi?id=20527

-- 

DAS-NETZWERKTEAM
mike gabriel, herweg 7, 24357 fleckeby
fon: +49 (1520) 1976 148

GnuPG Key ID 0x25771B31
mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de

freeBusy:
https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xfb

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

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

Message #33 received at 333@bugs.x2go.org (full text, mbox, reply):

From: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>

To: 333@bugs.x2go.org

Subject: Users can inject arbitrary data into X2Go Client via .bashrc

Date: Tue, 29 Oct 2013 12:36:14 +0000

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

Hi All,

Dan Halbert made me aware of it being easily possible to inject  
arbitrary data into X2Go Client via the server-side .bashrc file. This  
surely is a security problem in X2Go.

Thus, I found that we really need to do some sanity checks on incoming  
output from X2Go Servers to avoid such injections.

The idea is to invoke the server-side command with a UUID hash before  
and after the actuall command invocation:

1. execute server-side command from X2Go Client:

ssh <user>@<server> sh -c "echo <uuidhash> && <x2gocmd> && echo <uuidhash>

2. read data from X2Go Server:

X2GODATABEGIN:<uuidhash>
<x2godata_line1>
<x2godata_line2>
....
<x2godata_lineN>
X2GODATAEND:<uuidhash>

3. cut out the X2Go data returned by the server (in C++):

      QString begin_marker = "X2GODATABEGIN:"+uuid+"\n";
      QString end_marker = "X2GODATAEND:"+uuid+"\n";
      int output_begin=stdOutString.indexOf(begin_marker) + \\
                       begin_marker.length();
      int output_end=stdOutString.indexOf(end_marker);
      output = stdOutString.mid(output_begin, \\
                                output_end-output_begin);


I have a patch locally for this and will commit it in a minute. We can  
discuss the patch and move on from there when it's there.

Unfortunately, this patch does not fix #327 as it is impossible to use  
scp with echoing .bashrc files. With this patch applied, the session  
starts, but setting up the SSHfs shares fails with locking up X2Go  
Client.

For people who depend on echoing .bashrc files, please read my last  
post on #327.

Mike
-- 

DAS-NETZWERKTEAM
mike gabriel, herweg 7, 24357 fleckeby
fon: +49 (1520) 1976 148

GnuPG Key ID 0x25771B31
mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de

freeBusy:
https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xfb

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

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

Message #38 received at 333@bugs.x2go.org (full text, mbox, reply):

From: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>

To: 333-submitter@bugs.x2go.org

Cc: control@bugs.x2go.org, 333@bugs.x2go.org

Subject: X2Go issue (in src:x2goclient) has been marked as pending for release

Date: Tue, 29 Oct 2013 13:37:33 +0100 (CET)

tag #333 pending
fixed #333 4.0.1.2
thanks

Hello,

X2Go issue #333 (src:x2goclient) reported by you has been
fixed in X2Go Git. You can see the changelog below, and you can
check the diff of the fix at:

    http://code.x2go.org/gitweb?p=x2goclient.git;a=commitdiff;h=c121b7e

The issue will most likely be fixed in src:x2goclient (4.0.1.2).

light+love
X2Go Git Admin (on behalf of the sender of this mail)

---
commit c121b7e2d3d83abdc2d7a29637bc3294e38b2ec3
Author: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>
Date:   Tue Oct 29 13:36:58 2013 +0100

    Perform sanity checks on data that comes in from X2Go Servers. Prohibit the execution of arbitrary code via the ~/.bashrc file. (Fixes: #333).

diff --git a/debian/changelog b/debian/changelog
index e484ba5..e069591 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -8,6 +8,9 @@ x2goclient (4.0.1.2-0~x2go2) UNRELEASED; urgency=low
     + Store broker HTTPS certificate exceptions in
       $HOME/.x2go/ssl/exceptions (before: $HOME/ssl/exceptions).
       (Fixes: #328).
+    + Perform sanity checks on data that comes in from X2Go Servers.
+      Prohibit the execution of arbitrary code via the ~/.bashrc file.
+      (Fixes: #333).
   * Pull-in packaging changes from Debian.
 
   [ Ricardo Díaz Martín ]

Message #50 received at 333@bugs.x2go.org (full text, mbox, reply):

From: Dan Halbert <halbert@halwitz.org>

To: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>, 333@bugs.x2go.org

Cc: 333@bugs.x2go.org

Subject: Re: Bug#333: X2Go issue (in src:x2goclient) has been marked as pending for release

Date: Tue, 29 Oct 2013 08:59:30 -0400

Hi Mike, this fix to authenticate the commands is good. I didn't realize 
I was uncovering a security problem.

One question: the underlying crash was due to bad data. If authenticated 
but still bad data is sent, will the client still crash? I am thinking 
about a malicious server crafting something to crash the client or have 
it do something bad. I looked at the code diff and I didn't see some 
underlying verification of the x2go commands.

E.g.:
X2GODATABEGIN:<good-uuidhash>
bad data here
X2GODATAEND:<good-uuidhash>

Message #55 received at 333@bugs.x2go.org (full text, mbox, reply):

From: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>

To: Dan Halbert <halbert@halwitz.org>

Cc: 333@bugs.x2go.org

Subject: Re: Bug#333: X2Go issue (in src:x2goclient) has been marked as pending for release

Date: Tue, 29 Oct 2013 13:15:48 +0000

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

Hi Dan,

On  Di 29 Okt 2013 13:59:30 CET, Dan Halbert wrote:

> Hi Mike, this fix to authenticate the commands is good. I didn't  
> realize I was uncovering a security problem.
>
> One question: the underlying crash was due to bad data. If  
> authenticated but still bad data is sent, will the client still  
> crash? I am thinking about a malicious server crafting something to  
> crash the client or have it do something bad. I looked at the code  
> diff and I didn't see some underlying verification of the x2go  
> commands.
>
> E.g.:
> X2GODATABEGIN:<good-uuidhash>
> bad data here
> X2GODATAEND:<good-uuidhash>

I would indeed call this work in progress. See #334 for the ,,bad data  
here'' location you address above.

We surely need a means to ensure that the data sent over the wire is  
sane. An idea could be to encrypt/decrypt the data asymmetrically.  
Maybe something else...

Hmmm...

I don't think that evaluating the data in itself (via regexp e.g.)  
will lead to good results. We should invent a method that is common to  
all sorts of text data and makes sure that the data is for the client  
that requested it.

On the other hand... If you cannot trust your admin, who can you trust???

Any contribution of ideas is welcome.

Mike
-- 

DAS-NETZWERKTEAM
mike gabriel, herweg 7, 24357 fleckeby
fon: +49 (1520) 1976 148

GnuPG Key ID 0x25771B31
mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de

freeBusy:
https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xfb

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

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

Message #60 received at 333@bugs.x2go.org (full text, mbox, reply):

From: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>

To: 333@bugs.x2go.org, control@bugs.x2go.org

Subject: Re: [X2Go-Dev] Bug#333: Users can inject arbitrary data into X2Go Client via .bashrc

Date: Tue, 29 Oct 2013 13:41:47 +0000

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

clone #333 -1
reassign -1 python-x2go
retitle -1 Users can inject arbitrary data into Pyhoca-GUI via .bashrc
thanks

Hi All,

On  Di 29 Okt 2013 13:36:14 CET, Mike Gabriel wrote:

> Hi All,
>
> Dan Halbert made me aware of it being easily possible to inject  
> arbitrary data into X2Go Client via the server-side .bashrc file.  
> This surely is a security problem in X2Go.
>
> Thus, I found that we really need to do some sanity checks on  
> incoming output from X2Go Servers to avoid such injections.
>
> The idea is to invoke the server-side command with a UUID hash  
> before and after the actuall command invocation:
>
> 1. execute server-side command from X2Go Client:
>
> ssh <user>@<server> sh -c "echo <uuidhash> && <x2gocmd> && echo <uuidhash>
>
> 2. read data from X2Go Server:
>
> X2GODATABEGIN:<uuidhash>
> <x2godata_line1>
> <x2godata_line2>
> ....
> <x2godata_lineN>
> X2GODATAEND:<uuidhash>
>
> 3. cut out the X2Go data returned by the server (in C++):
>
>       QString begin_marker = "X2GODATABEGIN:"+uuid+"\n";
>       QString end_marker = "X2GODATAEND:"+uuid+"\n";
>       int output_begin=stdOutString.indexOf(begin_marker) + \\
>                        begin_marker.length();
>       int output_end=stdOutString.indexOf(end_marker);
>       output = stdOutString.mid(output_begin, \\
>                                 output_end-output_begin);
>
>
> I have a patch locally for this and will commit it in a minute. We  
> can discuss the patch and move on from there when it's there.
>
> Unfortunately, this patch does not fix #327 as it is impossible to  
> use scp with echoing .bashrc files. With this patch applied, the  
> session starts, but setting up the SSHfs shares fails with locking  
> up X2Go Client.
>
> For people who depend on echoing .bashrc files, please read my last  
> post on #327.
>
> Mike

This actually also applies to Python X2Go.

Mike
-- 

DAS-NETZWERKTEAM
mike gabriel, herweg 7, 24357 fleckeby
fon: +49 (1520) 1976 148

GnuPG Key ID 0x25771B31
mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de

freeBusy:
https://mail.das-netzwerkteam.de/freebusy/m.gabriel%40das-netzwerkteam.de.xfb

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

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

Message #67 received at 333@bugs.x2go.org (full text, mbox, reply):

From: Mike Gabriel <mike.gabriel@das-netzwerkteam.de>

To: 333-submitter@bugs.x2go.org

Cc: control@bugs.x2go.org, 333@bugs.x2go.org

Subject: X2Go issue (in src:x2goclient) has been marked as closed

Date: Tue, 17 Dec 2013 15:55:21 +0100 (CET)

close #333
thanks

Hello,

we are very hopeful that X2Go issue #333 reported by you
has been resolved in the new release (4.0.1.2) of the
X2Go source project »src:x2goclient«.

You can view the complete changelog entry of src:x2goclient (4.0.1.2)
below, and you can use the following link to view all the code changes
between this and the last release of src:x2goclient.

    http://code.x2go.org/gitweb?p=x2goclient.git;a=commitdiff;h=34591fd62844b2b955e6a4bf3cf44d4759c5e44c;hp=d5ff7886ae22a1e36541570e7095fac9860af6e8

If you feel that the issue has not been resolved satisfyingly, feel
free to reopen this bug report or submit a follow-up report with
further observations described based on the new released version
of src:x2goclient.

Thanks a lot for contributing to X2Go!!!

light+love
X2Go Git Admin (on behalf of the sender of this mail)

---
X2Go Component: src:x2goclient
Version: 4.0.1.2-0x2go2
Status: RELEASE
Date: Tue, 17 Dec 2013 15:21:38 +0100
Fixes: 139 230 241 311 315 316 328 333
Changes: 
 x2goclient (4.0.1.2-0x2go2) RELEASED; urgency=low
 .
   [ Mike Gabriel ]
   * New upstream version (4.0.1.2):
     - Provide Keywords: key in .desktop file.
     - Add NSIS packaging files for win32 builds to source tree.
       (Files provided by Oleksandr Shneyder, thanks!!!).
     - Rename win32 desktop and startmenu icon from "X2goClient" to "X2Go
       Client".
     - Store broker HTTPS certificate exceptions in
       $HOME/.x2go/ssl/exceptions (before: $HOME/ssl/exceptions).
       (Fixes: #328).
     - Perform sanity checks on data that comes in from X2Go Servers.
       Prohibit the execution of arbitrary code via the ~/.bashrc file.
       (Fixes: #333).
     - Add option --broker-cacertfile. Allow usage of non-system-wide
       installed (self-signed) SSL certificate chains for https (SSL)
       session broker connections. (Fixes: #311).
     - Update man page for new --tray-icon cmdline option.
     - Update man page for --broker-url. Explain the syntax of <URL>.
     - Properly handle (=expand) the "~" character in key filenames. (Brought to
       attention by Eldamir on IRC. Thanks!).
     - Expand tilde operator for all other file paths handed over to X2Go Client
       via sessions file or cmdline parameter.
     - Syntax fix of x2goclient.desktop file.
     - Test for various file locations of the pulseaudio cookie file.
     - Allow patching of qmake-qt4 executable path in Makefile.
     - Make qmake-qt4 and lrelease path in Makefile easily replacable (as
       RHEL-5 does not have those tools in $PATH).
     - Make sure that build_client and build_plugin are not build with parallel
       make.
     - Make x2goplugin-provider installable via Makefile.
   * Pull-in packaging changes from Debian.
   * debian/source/format:
     + Switch to format 1.0.
   * x2goclient.spec:
     + Ship x2goclient.spec (RPM package definitions) in upstream project.
       (Thanks to the Fedora package maintainers).
     + Clear (Fedora package) changelog.
     + Make package build on Fedora/EPEL versions that do not have the
       qtbrowserplugin package.
     + For EPEL-5 builds: replace full path to qmake-qt4 and lrelease.
     + Split up package into bin:packages: x2goclient, x2goplugin,
       x2goplugin-provider.
     + Make sure lrelease-qt4 is executed (not just lrelease).
 .
   [ Ricardo Díaz Martín ]
   * New upstream versino (4.0.1.2):
     - Strip whitespaces off of user name, host name and other
       strings when loading / saving session profiles.(Fixes: #315).
     - New option --tray-icon. Force showing the tray icon, even for
       hidden sessions. Also allow creation of .desktop files with
       --tray-icon optionally being enabled. (Fixes: #316).
     - Update Spanish translation.
 .
   [ Oleksandr Shneyder ]
   * New upstream version (4.0.1.2):
     - Support for keys "shadowuser" "shadowdisplay" and "shadowmode" in
       config file. This allows choosing the default display for shadow
       sessions.
     - Support for GSSApi(Kerberos 5) authentication. Using ssh/scp commands
       on Linux and Mac and plink/pscp on Windows.
     - Support for ChallengeResponseAuthentication (Google Authenticator)
     - Setting main window focus on mac (Fixes: #139).
     - Additional check if authentication with GSSApi successfull
     - c121b7e2d3d83abdc2d7a29637bc3294e38b2ec3 broke checking if remote
       command produce only stderr and not stdout. It made x2goclient crash
       if x2gostartagent send LIMIT error. Current commit fixes this issue.
     - SshMasterConnection should use current user name if no user name is
       specified in session settings
     - GSSApi(Kerberos 5) authentication for sshproxy and sshbroker
     - fixed GSSApi(Kerberos 5) authentication for sshproxy and sshbroker
       on windows
 .
   [ Heinrich Schuchardt ]
   * New upstream version (4.0.1.2):
     - Handle SSH host key changes more elegantly and allow user interaction
       if such a host key change occurs. (Fixes: #241).
 .
   [ Michael DePaulo ]
   * New upstream version (4.0.1.2):
     - win32: Add uninstall information to Add/Remove Programs. (Fixes: #230).

Send a report that this bug log contains spam.