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.
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.
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.
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.
Thanks, //richard