Bug 434557

Summary: KXMessages::broadcastMessageX triggers deprecated QByteArray behavior
Product: [Frameworks and Libraries] frameworks-kwindowsystem Reporter: Nicolas Fella <nicolas.fella>
Component: generalAssignee: KWin default assignee <kwin-bugs-null>
Status: RESOLVED FIXED    
Severity: normal CC: a.samirh78, kde, vlad.zahorodnii
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

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