Skip to content

Commit

Permalink
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

Co-authored-by: Thomas Lange <[email protected]>
  • Loading branch information
Krzmbrzl and radioactiveman committed Jan 21, 2024
1 parent 4fa4c65 commit 41fac28
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 41fac28

Please sign in to comment.