Subject: Bug#705: client sends password to http broker without percent encoding special characters such as &

From: Jason Alavaliant <[email-masked]>
Date: Wed, 17 Dec 2014 11:45:02 +1300

Package: x2goclient
Version: 4.0.3.1
Severity: grave
Tags: patch

I've just setup an x2go load balanced setup using x2gobroker (http 
connection - x2goclient --broker-url=http://server:8080/plain/inifile),  
   after putting it into production we found a number of our users had 
their passwords rejected when trying to sign into the x2go client to 
access the broker.

Tracing through the traffic/logs   we found that the problem is that 
password values were being set unencoded to the broker,   so for example 
if there was an & present in a password the form data was submitted in 
the form of

task=listsessions&user=user&password=mypass&word&authid=

which resulted in the data being read by the server as the pasword being 
mypass   rather than  mypass&word

The attached patch in my testing (done on Linux) fixes the client so 
data is correctly escaped so the above example would be submitted as

task=listsessions&user=user&password=mypass%26word&authid=

which is correctly parsed as the password being mypass&word
and allows the login to work.

If we could get an indication of when this fix is likely to make a 
client release it would appreciated since we currently don't have 
Windows and OSX builds with the patch and are trying to workout if it's 
worth the time of setting up development workstations to be able to 
compile the client for those platforms vs just waiting for the next 
client release.

Thanks for your time.
Jason

--- new/httpbrokerclient.cpp	2014-12-17 09:53:59.412566206 +1300
+++ old/httpbrokerclient.cpp	2014-12-17 09:30:27.733752000 +1300
@@ -251,8 +251,8 @@
         QString req;
         QTextStream ( &req ) <<
                              "task=listsessions&"<<
-                             "user="<<brokerUser<<"&"<<
-                             "password="<<config->brokerPass<<"&"<<
+                             "user="<<QUrl::toPercentEncoding(brokerUser)<<"&"<<
+                             "password="<<QUrl::toPercentEncoding(config->brokerPass)<<"&"<<
                              "authid="<<nextAuthId;
 
         x2goDebug << "sending request: "<< req.toUtf8();
@@ -290,8 +290,8 @@
         QTextStream ( &req ) <<
                              "task=selectsession&"<<
                              "sid="<<session<<"&"<<
-                             "user="<<brokerUser<<"&"<<
-                             "password="<<config->brokerPass<<"&"<<
+                             "user="<<QUrl::toPercentEncoding(brokerUser)<<"&"<<
+                             "password="<<QUrl::toPercentEncoding(config->brokerPass)<<"&"<<
                              "authid="<<nextAuthId;
         x2goDebug << "sending request: "<< req.toUtf8();
         QNetworkRequest request(QUrl(config->brokerurl));
@@ -324,9 +324,9 @@
         QString req;
         QTextStream ( &req ) <<
                              "task=setpass&"<<
-                             "newpass="<<newPass<<"&"<<
-                             "user="<<brokerUser<<"&"<<
-                             "password="<<config->brokerPass<<"&"<<
+                             "newpass="<<QUrl::toPercentEncoding(newPass)<<"&"<<
+                             "user="<<QUrl::toPercentEncoding(brokerUser)<<"&"<<
+                             "password="<<QUrl::toPercentEncoding(config->brokerPass)<<"&"<<
                              "authid="<<nextAuthId;
         x2goDebug << "sending request: "<< req.toUtf8();
         QNetworkRequest request(QUrl(config->brokerurl));