Skip to content

Commit

Permalink
Merge PR #6307: FIX(client): Crash when opening connect dialog
Browse files Browse the repository at this point in the history
When selecting new public servers, we randomize their order. The random number used for that is an unsigned integer from which we compute the modulus with the list size in order to get a random index into the list.

This involved an implicit conversion from an unsigned integer (the random number) to a signed integer (the list index - as Qt uses signed integers for that). Thus, b5a67c0 added an explicit cast to a signed integer. However, it did this to the random number before the modulus operation, meaning that if a large enough random number is drawn, the cast would actually create a negative integer, yielding a negative list index after the modulus. This caused a crash of the application.

The fix consists in switching to using the bounded() function that has an overload for signed integers and which can directly generate numbers in the desired range, making all of the above superfluous.

Fixes #6306
  • Loading branch information
Krzmbrzl authored Jan 21, 2024
2 parents 4fa4c65 + 41fac28 commit 910c2aa
Showing 1 changed file with 2 additions and 10 deletions.
12 changes: 2 additions & 10 deletions src/mumble/ConnectDialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@
# include <shlobj.h>
#endif

#if QT_VERSION >= QT_VERSION_CHECK(5, 10, 0)
# include <QRandomGenerator>
#endif
#include <QRandomGenerator>

#include <algorithm>

Expand Down Expand Up @@ -1466,13 +1464,7 @@ void ConnectDialog::fillList() {
}

while (!ql.isEmpty()) {
#if QT_VERSION >= QT_VERSION_CHECK(5, 10, 0)
ServerItem *si = static_cast< ServerItem * >(
ql.takeAt(static_cast< int >(QRandomGenerator::global()->generate()) % ql.count()));
#else
// Qt 5.10 introduces the QRandomGenerator class and in Qt 5.15 qrand got deprecated in its favor
ServerItem *si = static_cast< ServerItem * >(ql.takeAt(qrand() % ql.count()));
#endif
ServerItem *si = static_cast< ServerItem * >(ql.takeAt(QRandomGenerator::global()->bounded(0, ql.count())));
qlNew << si;
qlItems << si;
}
Expand Down

0 comments on commit 910c2aa

Please sign in to comment.