Skip to content

Commit a4a57b9

Browse files
committed
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.
1 parent 790d164 commit a4a57b9

3 files changed

Lines changed: 9 additions & 5 deletions

File tree

Viewer/ecflowUI/src/TextPager/TextPagerEdit.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "TextPagerEdit.hpp"
1616

1717
#include <algorithm>
18+
#include <array>
1819
#include <cmath>
1920

2021
#include <QGuiApplication>
@@ -72,9 +73,11 @@ TextPagerEdit::TextPagerEdit(QWidget* parent)
7273
};
7374

7475
std::array shortcuts = {shortcuts_t{tr("Copy"), SLOT(copy()), QKeySequence::Copy},
75-
shortcuts_t{tr("Select All"), SLOT(selectAll()), QKeySequence::SelectAll},
76-
shortcuts_t{QString(), nullptr, QKeySequence::UnknownKey}};
77-
for (int i = 0; shortcuts[i].member; ++i) {
76+
shortcuts_t{tr("Select All"), SLOT(selectAll()), QKeySequence::SelectAll}};
77+
static_assert(std::tuple_size_v<decltype(shortcuts)> == TextPagerEdit::ActionCount,
78+
"shortcuts must hold exactly one entry per ActionType");
79+
80+
for (std::size_t i = 0; i < shortcuts.size(); ++i) {
7881
d->actions[i] = new QAction(shortcuts[i].text, this);
7982
d->actions[i]->setShortcutContext(Qt::WidgetShortcut);
8083
d->actions[i]->setShortcut(QKeySequence(shortcuts[i].key));

Viewer/ecflowUI/src/TextPager/TextPagerEdit.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,8 @@ class TextPagerEdit : public QAbstractScrollArea, public VPropertyObserver {
153153
void setShowLineNumbers(bool b);
154154
void setLineNumberArea(TextPagerLineNumberArea* a);
155155

156-
enum ActionType { CopyAction, SelectAllAction };
156+
/// ActionCount must remain the last enumerator: it defines the size of TextEditPrivate::actions.
157+
enum ActionType { CopyAction, SelectAllAction, ActionCount };
157158
QAction* action(ActionType type) const;
158159

159160
public Q_SLOTS:

Viewer/ecflowUI/src/TextPager/TextPagerEdit_p.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ class TextEditPrivate : public QObject, public TextPagerLayout {
9595
pendingTimeOut, autoScrollLines;
9696
bool readOnly, cursorVisible, blockScrollBarUpdate, updateScrollBarPageStepPending, inMouseEvent;
9797
QBasicTimer autoScrollTimer, cursorBlinkTimer;
98-
QAction* actions[TextPagerEdit::SelectAllAction];
98+
QAction* actions[TextPagerEdit::ActionCount];
9999
TextPagerSection* sectionPressed;
100100
TextPagerCursor textCursor, dragOverrideCursor;
101101
QBasicTimer tripleClickTimer;

0 commit comments

Comments
 (0)