From 2ddf567feced415b112c2b929c7216ac22866349 Mon Sep 17 00:00:00 2001 From: baldurk Date: Mon, 24 May 2021 16:51:43 +0100 Subject: [PATCH] Require explicit opt-in to enable process injection on windows * This option (in spite of large warnings) continues to be a pitfall for new and experienced users alike, trying to use process injection without good reason and getting into trouble when it breaks. --- qrenderdoc/Code/Interface/PersistantConfig.h | 8 +++++ qrenderdoc/Windows/Dialogs/SettingsDialog.cpp | 17 +++++++++ qrenderdoc/Windows/Dialogs/SettingsDialog.h | 1 + qrenderdoc/Windows/Dialogs/SettingsDialog.ui | 36 ++++++++++++++++--- qrenderdoc/Windows/MainWindow.cpp | 6 +++- 5 files changed, 63 insertions(+), 5 deletions(-) diff --git a/qrenderdoc/Code/Interface/PersistantConfig.h b/qrenderdoc/Code/Interface/PersistantConfig.h index b8ae0cb17..896d6b1aa 100644 --- a/qrenderdoc/Code/Interface/PersistantConfig.h +++ b/qrenderdoc/Code/Interface/PersistantConfig.h @@ -572,6 +572,14 @@ DECLARE_REFLECTION_STRUCT(BugReport); "Defaults to ``False``."); \ CONFIG_SETTING_VAL(public, bool, bool, AllowGlobalHook, false) \ \ + DOCUMENT( \ + "``True`` if process injection is enabled. Since it can often break and is almost always " \ + "not want users want to do. New users can get confused by it being there and go to it " \ + "first.\n" \ + "\n" \ + "Defaults to ``False``."); \ + CONFIG_SETTING_VAL(public, bool, bool, AllowProcessInject, false) \ + \ DOCUMENT( \ "A list of :class:`ShaderProcessingTool` detailing shader processing programs. The list " \ "comes in priority order, with earlier processors preferred over later ones.\n" \ diff --git a/qrenderdoc/Windows/Dialogs/SettingsDialog.cpp b/qrenderdoc/Windows/Dialogs/SettingsDialog.cpp index 3793010a6..20801cb5f 100644 --- a/qrenderdoc/Windows/Dialogs/SettingsDialog.cpp +++ b/qrenderdoc/Windows/Dialogs/SettingsDialog.cpp @@ -221,6 +221,7 @@ SettingsDialog::SettingsDialog(ICaptureContext &ctx, QWidget *parent) #endif ui->AllowGlobalHook->setChecked(m_Ctx.Config().AllowGlobalHook); + ui->AllowProcessInject->setChecked(m_Ctx.Config().AllowProcessInject); ui->EventBrowser_TimeUnit->setCurrentIndex((int)m_Ctx.Config().EventBrowser_TimeUnit); ui->EventBrowser_AddFake->setChecked(m_Ctx.Config().EventBrowser_AddFake); @@ -248,6 +249,12 @@ SettingsDialog::SettingsDialog(ICaptureContext &ctx, QWidget *parent) ui->globalHookLabel->setToolTip(disabledTooltip); } +// process injection is not supported on non-Windows +#if !defined(Q_OS_WIN32) + ui->injectProcLabel->setVisible(false); + ui->AllowProcessInject->setVisible(false); +#endif + m_Init = false; QObject::connect(ui->Font_GlobalScale->lineEdit(), &QLineEdit::returnPressed, this, @@ -390,6 +397,16 @@ void SettingsDialog::on_AllowGlobalHook_toggled(bool checked) m_Ctx.GetCaptureDialog()->UpdateGlobalHook(); } +void SettingsDialog::on_AllowProcessInject_toggled(bool checked) +{ + m_Ctx.Config().AllowProcessInject = ui->AllowProcessInject->isChecked(); + + m_Ctx.Config().Save(); + + if(m_Ctx.HasCaptureDialog()) + m_Ctx.GetCaptureDialog()->UpdateGlobalHook(); +} + void SettingsDialog::on_CheckUpdate_AllowChecks_toggled(bool checked) { m_Ctx.Config().CheckUpdate_AllowChecks = ui->CheckUpdate_AllowChecks->isChecked(); diff --git a/qrenderdoc/Windows/Dialogs/SettingsDialog.h b/qrenderdoc/Windows/Dialogs/SettingsDialog.h index 378b2d807..d2530309d 100644 --- a/qrenderdoc/Windows/Dialogs/SettingsDialog.h +++ b/qrenderdoc/Windows/Dialogs/SettingsDialog.h @@ -63,6 +63,7 @@ private slots: void on_saveDirectory_textEdited(const QString &save); void on_browseSaveCaptureDirectory_clicked(); void on_AllowGlobalHook_toggled(bool checked); + void on_AllowProcessInject_toggled(bool checked); void on_CheckUpdate_AllowChecks_toggled(bool checked); void on_Font_PreferMonospaced_toggled(bool checked); void on_AlwaysReplayLocally_toggled(bool checked); diff --git a/qrenderdoc/Windows/Dialogs/SettingsDialog.ui b/qrenderdoc/Windows/Dialogs/SettingsDialog.ui index 81fa666b4..fca851217 100644 --- a/qrenderdoc/Windows/Dialogs/SettingsDialog.ui +++ b/qrenderdoc/Windows/Dialogs/SettingsDialog.ui @@ -184,7 +184,7 @@ Since this is a global system hook it must be used carefully and only when neces - + If a capture is marked as being created on a significantly different system (different OS or platform) @@ -252,7 +252,7 @@ e.g. a value of 2 means 0 will display as 0.00, 0.5 as 0.50 - + Allows RenderDoc to phone home to https://renderdoc.org to anonymously check for new versions. @@ -294,7 +294,7 @@ Defaults to %TEMP%. - + If a capture is marked as being created on a significantly different system (different OS or platform) @@ -338,7 +338,7 @@ E.g. a value of 3 means 0.005 / 10 = 5E-4 - + Allows RenderDoc to phone home to https://renderdoc.org to anonymously check for new versions. @@ -363,6 +363,34 @@ Since this is a global system hook it must be used carefully and only when neces + + + + Enables the ability to inject into processes on windows. + +Injecting into processes can be unreliable and should only be used as a last resort when no other +methods succeed, it should not be used as a primary method of launching applications. Instead the +program should be launched through RenderDoc via the Launch Process panel. + + + Enable process injection (restart required) + + + + + + + Enables the ability to inject into processes on windows. + +Injecting into processes can be unreliable and should only be used as a last resort when no other +methods succeed, it should not be used as a primary method of launching applications. Instead the +program should be launched through RenderDoc via the Launch Process panel. + + + + + + diff --git a/qrenderdoc/Windows/MainWindow.cpp b/qrenderdoc/Windows/MainWindow.cpp index 6309ac174..595193e25 100644 --- a/qrenderdoc/Windows/MainWindow.cpp +++ b/qrenderdoc/Windows/MainWindow.cpp @@ -115,7 +115,11 @@ MainWindow::MainWindow(ICaptureContext &ctx) : QMainWindow(NULL), ui(new Ui::Mai setProperty("ICaptureContext", QVariant::fromValue((void *)&ctx)); -#if !defined(Q_OS_WIN32) +#if defined(Q_OS_WIN32) + // remove inject menu item when it's not enabled in the settings + if(!ctx.Config().AllowProcessInject) + ui->menu_File->removeAction(ui->action_Inject_into_Process); +#else // process injection is not supported on non-Windows, so remove the menu item rather than disable // it without a clear way to communicate that it is never supported ui->menu_File->removeAction(ui->action_Inject_into_Process);