Bug 434557 - KXMessages::broadcastMessageX triggers deprecated QByteArray behavior
Summary: KXMessages::broadcastMessageX triggers deprecated QByteArray behavior
Status: RESOLVED FIXED
Alias: None
Product: frameworks-kwindowsystem
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-03-17 18:39 UTC by Nicolas Fella
Modified: 2021-12-17 15:11 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Fella 2021-03-17 18:39:34 UTC
SUMMARY

In https://invent.kde.org/frameworks/kwindowsystem/-/blob/master/src/platforms/xcb/kxmessages.cpp#L331 msg can be accesses out of bounds (due to the pos <= len above). In Qt5 this causes a nullbyte to be read, but that behavior is deprecated (https://doc.qt.io/qt-5/qbytearray.html#operator-5b-5d), causing a runtime warning: Using QByteRef with an index pointing outside the valid range of a QByteArray. The corresponding behavior is deprecated, and will be changed in a future version of Qt

In Qt6 this results in an assert 

STEPS TO REPRODUCE
1. Run kxmessages_unittest against Qt6 or put Q_ASSERT(i+pos < msg.length()) in the code

OBSERVED RESULT
An assert triggers

EXPECTED RESULT
No assert

SOFTWARE/OS VERSIONS
KDE Frameworks Version: master 
Qt Version: Current dev or 5.15 + manual assert
Comment 1 Bug Janitor Service 2021-12-16 22:32:32 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kwindowsystem/-/merge_requests/40
Comment 2 Fabian Vogt 2021-12-17 15:11:41 UTC
Git commit 11eeb8369426a1f8cd805dadf5e67d95dc5feb65 by Fabian Vogt.
Committed on 16/12/2021 at 22:30.
Pushed by fvogt into branch 'master'.

Avoid using QByteArray::operator[] for the null terminator

While QByteArray has an implicit null terminator at the end, this is not
meant to be accessible through operator[]. In Qt 5, OOB accesses return 0
(and cause a warning in later versions), but in Qt 6 it's forbidden entirely.
Rewrite the code slightly to just avoid reading the null terminator
altogether. For completeness, this also zeros the remaining buffer when
Xlib events are used.

Also replace use of strlen with QByteArray::size().

M  +9    -6    src/platforms/xcb/kxmessages.cpp

https://invent.kde.org/frameworks/kwindowsystem/commit/11eeb8369426a1f8cd805dadf5e67d95dc5feb65