Fix freeze and high CPU usage on invalid STDIN data #1628
Conversation
phoerious
force-pushed
the
hotfix/1620-native-messaging-lockup
branch
from
d03d816
to
2f5357d
Compare
| arr.append(static_cast<char>(c)); | ||
| } | ||
|
|
||
| if (arr.length() > 0) { |
There was a problem hiding this comment.
Shouldn't we lock at the beginning of the function or the beginning of the sendReply function?
There was a problem hiding this comment.
Why lock the mutex at the beginning when the first time any members (that need locking) are used is at this point?
There was a problem hiding this comment.
I didn't change anything here, but yes, it's weird.
There was a problem hiding this comment.
So, I had a quick look at it. On *nix it should be perfectly safe to remove the mutex, but on Windows, there is a weird mix of threads and signal/slot connections. I don't have the time right now to investigate where it's really needed and where it isn't. I'd prefer we just leave it as is for now.
There was a problem hiding this comment.
Defer to the 2.4 epic to cleanup this code
phoerious
force-pushed
the
hotfix/1620-native-messaging-lockup
branch
from
2f5357d
to
e78fb78
Compare
- Fix unnecessary automatic upgrade to KDBX 4.0 and prevent challenge-response key being stripped [#1568] - Abort saving and show an error message when challenge-response fails [#1659] - Support inner stream protection on all string attributes [#1646] - Fix favicon downloads not finishing on some websites [#1657] - Fix freeze due to invalid STDIN data [#1628] - Correct issue with encrypted RSA SSH keys [#1587] - Fix crash on macOS due to QTBUG-54832 [#1607] - Show error message if ssh-agent communication fails [#1614] - Fix --pw-stdin and filename parameters being ignored [#1608] - Fix Auto-Type syntax check not allowing spaces and special characters [#1626] - Fix reference placeholders in combination with Auto-Type [#1649] - Fix qtbase translations not being loaded [#1611] - Fix startup crash on Windows due to missing SVG libraries [#1662] - Correct database tab order regression [#1610] - Fix GCC 8 compilation error [#1612] - Fix copying of advanced attributes on KDE [#1640] - Fix member initialization of CategoryListWidgetDelegate [#1613] - Fix inconsistent toolbar icon sizes and provide higher-quality icons [#1616] - Improve preview panel geometry [#1609]
Description
Fixes a freeze and high CPU usage when provided with invalid STDIN data and/or a wrong database filename.
Resolves #1620
How has this been tested?
Manually. The reproduction examples in #1620 don't freeze KeePassXC anymore.
Types of changes
Checklist:
-DWITH_ASAN=ON. [REQUIRED]