On Thu, Mar 30, 2017 at 8:50 AM, Mihai Moldovan <ionic@ionic.de> wrote:
On 26.03.2017 06:11 PM, git-admin@x2go.org wrote:
This is an automated email from the git hooks/post-receive script.
x2go pushed a commit to branch bugfix/1100 in repository x2goclient.
commit a7ed6868825c111f8d0fa4a64aa82115b8dab039 Author: Mike DePaulo <mikedep333@gmail.com> Date: Sun Mar 26 12:09:11 2017 -0400
Don't override PATH for the actual session or application command. Fixes: #1100
debian/changelog | 3 ++ src/onmainwindow.cpp | 67 ++++++++++++++++++++++++++++----------------- src/onmainwindow.h | 1 + src/sshmasterconnection.cpp | 4 +-- src/sshmasterconnection.h | 2 +- src/sshprocess.cpp | 15 ++++++++-- src/sshprocess.h | 2 +- 7 files changed, 62 insertions(+), 32 deletions(-)
diff --git a/debian/changelog b/debian/changelog index 42c1e52..8e71aeb 100644 --- a/debian/changelog +++ b/debian/changelog @@ -180,6 +180,9 @@ x2goclient (4.1.0.1-0x2go1) UNRELEASED; urgency=medium default because the installation dir is not writeable by users) + CVE-2017-6542 was fixed
- Don't override PATH for the actual session or application
command.Fixes: #1100
- New upstream version (4.1.0.1): diff --git a/src/onmainwindow.cpp b/src/onmainwindow.cpp index 5dd3906..0b15649 100644 --- a/src/onmainwindow.cpp +++ b/src/onmainwindow.cpp @@ -6291,7 +6291,7 @@ void ONMainWindow::slotProxyStderr() { xmodExecuted=true; QTimer::singleShot (
2000, this,
4000, this,Why are you changing this? It's orthogonal to all the other changes.
I assumed this needs to be executed after a 2000ms delay, because it takes up to 2000ms for x2goruncommand to run. Since we are introducing an additional command over SSH, I added another 2000ms.
SLOT ( slotExecXmodmap() ) ); } }@@ -6700,6 +6700,39 @@ void ONMainWindow::slotAppDialog()
void ONMainWindow::runCommand() { +
- if ( runRemoteCommand )
- {
/* 1st override PATH and determine the base path to x2goruncommand.Whitespace errors, you're mixing tabs and spaces here.
Ack.
* Then in SlotRunCommand, call x2goruncommand without overriding PATH.* This ensures that the PATH is never overriden with for the actual"overridden", remove "with" or specify.
Ack.
* user session.* Fixes: #1100*/sshConnection->executeCommand ( "x2gobasepath", this,SLOT ( SlotRunCommand ( bool,QString,int )), true);- } +#ifdef Q_WS_HILDON
- //wait 5 seconds and execute xkbcomp
- QTimer::singleShot ( 5000, this, SLOT ( slotExecXmodmap() ) ); +#endif
We'll probably execute slotExecXmodmap a bit sooner than before with that change (since you're not doing it in SlotRunCommand - but that wouldn't work for XDMCP and Shadow sessions, since they don't set runRemoteCommand to true.)
It's fine, I guess. I just noticed that the code in question is broken anyway - it wouldn't even compile on HILDON anymore, so let's ignore that.
Otherwise looks good.
Mihai
Thank you for the review. -Mike