From a4a57b9943c44ac6dcaf3551d09affdde9eb677a Mon Sep 17 00:00:00 2001 From: Marcos Bento Date: Thu, 16 Jul 2026 18:37:05 +0100 Subject: [PATCH] Fix out-of-bounds write in TextPagerEdit TextEditPrivate::actions was sized with the enumerator value SelectAllAction (i.e. 1) rather than an element count, leaving room for a single QAction while the constructor initialises two. The write to actions[1] hit the adjacent sectionPressed member; being undefined behaviour, at -O0 it merely corrupted that member, while at -O2 GCC 14 used the single-element bound to constrain the loop index and broke the loop exit condition, making ecFlowUI segfault whenever a TextPagerEdit was constructed, as is the case with Panels > Add info panel. Introduce ActionCount and size the array with it, replace the null-sentinel loop with a bounded loop, and tie the shortcut table to ActionType with a static_assert. --- Viewer/ecflowUI/src/TextPager/TextPagerEdit.cpp | 9 ++++++--- Viewer/ecflowUI/src/TextPager/TextPagerEdit.hpp | 3 ++- Viewer/ecflowUI/src/TextPager/TextPagerEdit_p.hpp | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Viewer/ecflowUI/src/TextPager/TextPagerEdit.cpp b/Viewer/ecflowUI/src/TextPager/TextPagerEdit.cpp index 873b00274..2ee10d71f 100644 --- a/Viewer/ecflowUI/src/TextPager/TextPagerEdit.cpp +++ b/Viewer/ecflowUI/src/TextPager/TextPagerEdit.cpp @@ -15,6 +15,7 @@ #include "TextPagerEdit.hpp" #include +#include #include #include @@ -72,9 +73,11 @@ TextPagerEdit::TextPagerEdit(QWidget* parent) }; std::array shortcuts = {shortcuts_t{tr("Copy"), SLOT(copy()), QKeySequence::Copy}, - shortcuts_t{tr("Select All"), SLOT(selectAll()), QKeySequence::SelectAll}, - shortcuts_t{QString(), nullptr, QKeySequence::UnknownKey}}; - for (int i = 0; shortcuts[i].member; ++i) { + shortcuts_t{tr("Select All"), SLOT(selectAll()), QKeySequence::SelectAll}}; + static_assert(std::tuple_size_v == TextPagerEdit::ActionCount, + "shortcuts must hold exactly one entry per ActionType"); + + for (std::size_t i = 0; i < shortcuts.size(); ++i) { d->actions[i] = new QAction(shortcuts[i].text, this); d->actions[i]->setShortcutContext(Qt::WidgetShortcut); d->actions[i]->setShortcut(QKeySequence(shortcuts[i].key)); diff --git a/Viewer/ecflowUI/src/TextPager/TextPagerEdit.hpp b/Viewer/ecflowUI/src/TextPager/TextPagerEdit.hpp index bbe3fa65a..1aaeeac30 100644 --- a/Viewer/ecflowUI/src/TextPager/TextPagerEdit.hpp +++ b/Viewer/ecflowUI/src/TextPager/TextPagerEdit.hpp @@ -153,7 +153,8 @@ class TextPagerEdit : public QAbstractScrollArea, public VPropertyObserver { void setShowLineNumbers(bool b); void setLineNumberArea(TextPagerLineNumberArea* a); - enum ActionType { CopyAction, SelectAllAction }; + /// ActionCount must remain the last enumerator: it defines the size of TextEditPrivate::actions. + enum ActionType { CopyAction, SelectAllAction, ActionCount }; QAction* action(ActionType type) const; public Q_SLOTS: diff --git a/Viewer/ecflowUI/src/TextPager/TextPagerEdit_p.hpp b/Viewer/ecflowUI/src/TextPager/TextPagerEdit_p.hpp index 9094370d6..ee87c558c 100644 --- a/Viewer/ecflowUI/src/TextPager/TextPagerEdit_p.hpp +++ b/Viewer/ecflowUI/src/TextPager/TextPagerEdit_p.hpp @@ -95,7 +95,7 @@ class TextEditPrivate : public QObject, public TextPagerLayout { pendingTimeOut, autoScrollLines; bool readOnly, cursorVisible, blockScrollBarUpdate, updateScrollBarPageStepPending, inMouseEvent; QBasicTimer autoScrollTimer, cursorBlinkTimer; - QAction* actions[TextPagerEdit::SelectAllAction]; + QAction* actions[TextPagerEdit::ActionCount]; TextPagerSection* sectionPressed; TextPagerCursor textCursor, dragOverrideCursor; QBasicTimer tripleClickTimer;