Subject: Invalid parent widget cast in InteractionDialog

From: Timo Teras <[email-masked]>
Date: Mon, 8 Oct 2018 14:13:39 +0300

Package: x2goclient
Version: 4.1.2.1

Running x2goclient in valgrind results in the following invalid reads:

==24194== Invalid read of size 1
==24194==    at 0x1AD4BD: InteractionDialog::InteractionDialog(QWidget*) (InteractionDialog.cpp:40)
==24194==    by 0x16F9FB: ONMainWindow::ONMainWindow(QWidget*) (onmainwindow.cpp:462)
==24194==    by 0x1BF5F3: x2goMain (ongetpass.cpp:110)
==24194==    by 0x1D689C: fork_helper(int, char**) (x2goclient.cpp:89)
==24194==    by 0x130F66: main (x2goclient.cpp:123)
==24194==  Address 0x4a2ccd2 is 14 bytes before a block of size 488 alloc'd
==24194==    at 0x489DD28: operator new(unsigned long) (vg_replace_malloc.c:334)
==24194==    by 0x5F79687: QFrame::QFrame(QWidget*, QFlags<Qt::WindowType>) (in /usr/lib/libQt5Widgets.so.5.10.1)
==24194==    by 0x1ACDED: SVGFrame::SVGFrame(QString, bool, QWidget*, QFlags<Qt::WindowType>) (SVGFrame.cpp:29)
==24194==    by 0x16F44A: ONMainWindow::ONMainWindow(QWidget*) (onmainwindow.cpp:361)
==24194==    by 0x1BF5F3: x2goMain (ongetpass.cpp:110)
==24194==    by 0x1D689C: fork_helper(int, char**) (x2goclient.cpp:89)
==24194==    by 0x130F66: main (x2goclient.cpp:123)
==24194== 
==24194== Invalid read of size 1
==24194==    at 0x1AD6DE: InteractionDialog::InteractionDialog(QWidget*) (InteractionDialog.cpp:62)
==24194==    by 0x16F9FB: ONMainWindow::ONMainWindow(QWidget*) (onmainwindow.cpp:462)
==24194==    by 0x1BF5F3: x2goMain (ongetpass.cpp:110)
==24194==    by 0x1D689C: fork_helper(int, char**) (x2goclient.cpp:89)
==24194==    by 0x130F66: main (x2goclient.cpp:123)
==24194==  Address 0x4a2ccd2 is 14 bytes before a block of size 488 alloc'd
==24194==    at 0x489DD28: operator new(unsigned long) (vg_replace_malloc.c:334)
==24194==    by 0x5F79687: QFrame::QFrame(QWidget*, QFlags<Qt::WindowType>) (in /usr/lib/libQt5Widgets.so.5.10.1)
==24194==    by 0x1ACDED: SVGFrame::SVGFrame(QString, bool, QWidget*, QFlags<Qt::WindowType>) (SVGFrame.cpp:29)
==24194==    by 0x16F44A: ONMainWindow::ONMainWindow(QWidget*) (onmainwindow.cpp:361)
==24194==    by 0x1BF5F3: x2goMain (ongetpass.cpp:110)
==24194==    by 0x1D689C: fork_helper(int, char**) (x2goclient.cpp:89)
==24194==    by 0x130F66: main (x2goclient.cpp:123)

InteractionDialog::InteractionDialog incorrectly casts parent widget to
ONMainWindows (it is in reality of type SVGFrame).

I am not sure what is the preferred fix, but one option for the fix is
as follows:

diff --git a/src/InteractionDialog.cpp b/src/InteractionDialog.cpp
index 6ab5d45..4e6ff07 100644
--- a/src/InteractionDialog.cpp
+++ b/src/InteractionDialog.cpp
@@ -32,12 +32,10 @@
 #endif
 #endif
 
-InteractionDialog::InteractionDialog(QWidget* parent): SVGFrame(":/img/svg/passform.svg",
+InteractionDialog::InteractionDialog(QWidget* parent, int miniMode): SVGFrame(":/img/svg/passform.svg",
             false,parent )
 {
-    mw=(ONMainWindow*)parent;
-
-    if ( !mw->retMiniMode() )
+    if ( !miniMode )
         setFixedSize ( this->sizeHint().width(),this->sizeHint().height()*1.5 );
     else
         setFixedSize ( 310,280 );
@@ -59,7 +57,7 @@ InteractionDialog::InteractionDialog(QWidget* parent): SVGFrame(":/img/svg/passf
     pal.setColor ( QPalette::Base, QColor ( 255,255,255,255 ) );
 
     QFont fnt=this->font();
-    if ( mw->retMiniMode() )
+    if ( miniMode )
 #ifdef Q_WS_HILDON
         fnt.setPointSize ( 10 );
 #else
diff --git a/src/InteractionDialog.h b/src/InteractionDialog.h
index c0238eb..14cee44 100644
--- a/src/InteractionDialog.h
+++ b/src/InteractionDialog.h
@@ -32,7 +32,7 @@ class InteractionDialog: public SVGFrame
     Q_OBJECT
 public:
     enum IMode {SESSION,BROKER};
-    InteractionDialog ( QWidget* parent=0);
+    InteractionDialog ( QWidget* parent=0, int miniMode=0);
     virtual ~InteractionDialog();
     void reset();
     void appendText(QString txt);
@@ -46,7 +46,6 @@ public:
         return interactionMode;
     }
 private:
-    ONMainWindow* mw;
     QTextEdit* textEdit;
     QPushButton* cancelButton;
     QLineEdit* textEntry;
diff --git a/src/onmainwindow.cpp b/src/onmainwindow.cpp
index 52e765c..00c6eb2 100644
--- a/src/onmainwindow.cpp
+++ b/src/onmainwindow.cpp
@@ -460,7 +460,7 @@ ONMainWindow::ONMainWindow ( QWidget *parent ) :QMainWindow ( parent )
     initPassDlg();
     initSelectSessDlg();
     initStatusDlg();
-    interDlg=new InteractionDialog(bgFrame);
+    interDlg=new InteractionDialog(bgFrame, miniMode);
     connect(interDlg, SIGNAL(closeInterractionDialog()), this, SLOT(slotCloseInteractionDialog()));
     username->addWidget ( interDlg );