Hi all,

On Sa 18 Mai 2013 21:48:30 CEST Richard Weinberger wrote:

while reviewing x2go I've encountered issues which scared hell out of me. The client seems to perform zero input validation. A rough server
can easily crash the client and most likely execute arbitrary code. For example x2goSession ONMainWindow::getSessionFromString ( const
QString& string ), it is feed with input from the server.

QStringList lst=string.split ( '|' );
x2goSession s;
s.agentPid=lst[0];
s.sessionId=lst[1];
s.display=lst[2];
s.server=lst[3];
s.status=lst[4];
s.crTime=lst[5];
s.cookie=lst[6];
s.clientIp=lst[7];
s.grPort=lst[8];
s.sndPort=lst[9];

---

If a line from the server, does not enough "|" we end up with
out-of-bound array access. The source is full with such issues.

Can you please file a bug against X2Go Client, so that we do not loose
this on the list. Those issues have to fixed. Please mark them as grave:

To: submit@bugs.x2go.org Subject: <a-good-one> """ Package: x2goclient Version: 4.0.1.0 Severity: grave

<your-bug-description> """

Finally I've also looked at the server. In short, the 90's called, they want their setuid bugs back. x2gosqlitewrapper.c just wrong, anyone can make it executing
whatever binary he wants with higher privileges.

This one Richard and I have fixed during last night. The issues were
present in X2Go Server and the broker agent in X2Go Session Broker.
Please upgrade X2Go Server ( -> 4.0.0.2) and X2Go Session Broker ( ->
0.0.2.1). This is highly recommended!!!

But it's not only the code that worries me. On Windows the client executes per default sshd and x11. Both are
listening on all available IP-Addresses. You silently install a user "sshuser" on Windows, which has the
password of the currently logged in Windows user and give him a login shell.

Huuhhhh...

@Alex: this sounds wrong to me... isn't it possible to launch an SSH
daemon under the user's ID that is currently logged on (on some non-22
port)???

I haven't seen such a trainwreck of software for a long time. By installing it on my system you've successfully backdoor'ed my
clients and the server.

Let's continue working together to remove those trainwreck bits and
pieces and the X2Go possibly becomes suitable for you.

Improving 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.xf...

Hello Richard,

Am 18.05.2013 21:48, schrieb Richard Weinberger:

Hi x2go users/developers,

while reviewing x2go I've encountered issues which scared hell out of me. The client seems to perform zero input validation. A rough server can easily crash the client and most likely execute arbitrary code. For example x2goSession ONMainWindow::getSessionFromString ( const QString& string ), it is feed with input from the server.

QStringList lst=string.split ( '|' );
x2goSession s;
s.agentPid=lst[0];
s.sessionId=lst[1];
s.display=lst[2];
s.server=lst[3];
s.status=lst[4];
s.crTime=lst[5];
s.cookie=lst[6];
s.clientIp=lst[7];
s.grPort=lst[8];
s.sndPort=lst[9];

---

If a line from the server, does not enough "|" we end up with out-of-bound array access. The source is full with such issues.

You are right, it is possible, that X2Go Client can be crashed with the wrong output from the server. This issue could (and should) be easily fixed by replacing operator "[n]" with method "value(n)". However, I don't think, that this issue is so dramatic as you described it. Why some one should open a SSH/X2GO connection to "rough" server? I didn't see such use case yet, when an administrator of server want to crash the client application on a machine of his user. If a user root on your Linux system is not an evil person, who want crash the X2Go Client on your desktop, you should not worry about this issue. But if you living in the world of BOFH, please don't use the X2Go Client until this issue will be fixed. I'll fix it very soon.

Finally I've also looked at the server. In short, the 90's called, they want their setuid bugs back. x2gosqlitewrapper.c just wrong, anyone can make it executing whatever binary he wants with higher privileges.

Sorry, I don't understand what are you talking about. I not found the file "x2gosqlitewrapper.c" in the source tree of package "x2go server". If you found a security problem in the recent x2goserver code, please open a bug report on bug tracker, describe the problem and show how it can be used. In best case show an example of exploit and send a bug fix. Saying "it is just wrong, anyone can do something" is just your opinion without any arguments.

But it's not only the code that worries me. On Windows the client executes per default sshd and x11. Both are listening on all available IP-Addresses.

Yes, this components are required by X2Go Client. This services are configured by default to listen all IP-Adresses. It is possible to configure them to listen for connections only on localhost, but I see it just as "nice to have" feature. Starting this services is not creating backdoor on the system, otherwise most UNIX machines would be backdoor'ed, because they running same services. Furthermore, SSHD used by X2Go is running only with user privileges and opening an access for only one user and only shortly for each SSHFS connection. The rest time SSHD don't accept a SSH-connections. In addition, each Windows system have a firewall that by default configured to drop incoming TCP-connections. This make SSHD and X11 to be only accessible from localhost.

You silently install a user "sshuser" on Windows, which has the password of the currently logged in Windows user and give him a login shell.

This is so untrue! X2Go Client can not install users on Windows system. To be able to do something like that, X2Go Client must have an administrator privileges. All X2Go Client components running with user privileges. A SSHD open SSH access for current user and this is required for SSHFS, which used to export client directories to server. If you don't trust your server, just don't export your directories. And you should not do this, independent what kind of network FS are you using. It is always possible, that untrusted server can manipulate your data or credentials. It's impossible to open a SSH-Connection to your client until you don't exporting directories to server.

I haven't seen such a trainwreck of software for a long time. By installing it on my system you've successfully backdoor'ed my clients and the server.

I appreciated your criticism, but writing something like that in the ML of a community project is just not respecting the work of people, who spent a lot of their time and costs to develop something useful for others.

Alex

Thanks, //richard

---

X2Go-Dev mailing list X2Go-Dev@lists.berlios.de https://lists.berlios.de/mailman/listinfo/x2go-dev

-- Oleksandr Shneyder Dipl. Informatik X2go Core Developer Team

email: oleksandr.shneyder@obviously-nice.de web: www.obviously-nice.de

--> X2go - everywhere@home