XGetFCtl.c:268:71: warning: use of uninitialized value ‘*Feedback.length’
When building libXi with gcc 14.1 using -fanalyzer it reports:
XGetFCtl.c:268:71: warning: use of uninitialized value ‘*Feedback.length’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
268 | Feedback = (XFeedbackState *) ((char *)Feedback + Feedback->length);
| ~~~~~~~~^~~~~~~~The full walkthrough from gcc is pasted below, but the short summary is that if we somehow hit the default: case in the switch statement to fill in each member of the feedback array, which should only happen if the server returns a feedback item of a type the library doesn't know, then Feedback->length is never set, and advancing the pointer for the next item uses whatever value was left in the memory when it was malloc'ed.
While it seems simple to add Feedback->length = 0 to the default case to avoid using unknown values, I'm not sure that's safe, since then we'll return a feedback array to the caller that's missing an item, and which may cause the client to try to read uninitialized memory when they iterate over the returned set of items. We could avoid that by also adding *num_feedbacks-- ; i--; to the default case, reducing the count for the number of items returned to the caller, just ignoring the unknown item, but I don't know if that's a better option than just throwing away the list and returning an error to the caller.
XGetFCtl.c:268:71: warning: use of uninitialized value ‘*Feedback.length’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
268 | Feedback = (XFeedbackState *) ((char *)Feedback + Feedback->length);
| ~~~~~~~~^~~~~~~~
‘XGetFeedbackControl’: event 1
|
| 82 | if (_XiCheckExtInit(dpy, XInput_Initial_Release, info) == -1)
| | ^
| | |
| | (1) following ‘false’ branch...
|
‘XGetFeedbackControl’: event 2
|
|/usr/include/X11/Xlibint.h:539:32:
| 539 | req = (x##name##Req *) _XGetRequest(dpy, X_##name, sz)
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (2) ...to here
/usr/include/X11/Xlibint.h:551:9: note: in expansion of macro ‘GetReqSized’
| 551 | GetReqSized(name, SIZEOF(x##name##Req), req)
| | ^~~~~~~~~~~
XGetFCtl.c:85:5: note: in expansion of macro ‘GetReq’
| 85 | GetReq(GetFeedbackControl, req);
| | ^~~~~~
|
|
‘XGetFeedbackControl’: events 3-23
|
| 90 | if (!_XReply(dpy, (xReply *) & rep, 0, xFalse))
| | ^
| | |
| | (3) following ‘false’ branch...
|......
| 93 | if (rep.length > 0) {
| | ~~~~~~~~~~~
| | | |
| | | (4) ...to here
| | (5) following ‘true’ branch...
|......
| 98 | *num_feedbacks = rep.num_feedbacks;
| | ~~~~~~~~~~~~~~~~~
| | |
| | (6) ...to here
| 99 |
| 100 | if (rep.length < (INT_MAX >> 2)) {
| | ~
| | |
| | (7) following ‘true’ branch...
| 101 | nbytes = rep.length << 2;
| | ~~~~~~~~~~~~~~~
| | |
| | (8) ...to here
|......
| 104 | if (!f) {
| | ~
| | |
| | (9) following ‘false’ branch (when ‘f’ is non-NULL)...
|......
| 109 | end = (char *)f + nbytes;
| | ~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (10) ...to here
|......
| 112 | for (i = 0; i < *num_feedbacks; i++) {
| | ~~~~~~~~~~~~~~~~~~
| | |
| | (11) following ‘true’ branch...
| | (23) following ‘false’ branch...
| 113 | if ((char *)f + sizeof(*f) > end ||
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | | | |
| | | (12) ...to here (15) following ‘false’ branch...
| | (13) following ‘false’ branch...
| 114 | f->length == 0 || f->length > nbytes)
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | | | |
| | | | (16) ...to here
| | | (17) following ‘false’ branch...
| | (14) ...to here
| 115 | goto out;
| 116 | nbytes -= f->length;
| | ~~~~~~~~~~~~~~~~~~~
| | |
| | (18) ...to here
| 117 |
| 118 | switch (f->class) {
| | ~~~~~~
| | |
| | (19) following ‘default:’ branch...
|......
| 143 | default:
| | ~~~~~~~
| | |
| | (20) ...to here
|......
| 147 | if (size > INT_MAX)
| | ~
| | |
| | (21) following ‘false’ branch (when ‘size <= 2147483647’)...
| 148 | goto out;
| 149 | f = (xFeedbackState *) ((char *)f + f->length);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (22) ...to here
|
‘XGetFeedbackControl’: event 24
|
|/usr/include/X11/Xlibint.h:463:24:
| 463 | # define Xmalloc(size) malloc((size_t)(size))
| | ^~~~~~~~~~~~~~~~~~~~~~
| | |
| | (24) ...to here
XGetFCtl.c:152:20: note: in expansion of macro ‘Xmalloc’
| 152 | Feedback = Xmalloc(size);
| | ^~~~~~~
|
‘XGetFeedbackControl’: event 25
|
|/usr/include/X11/Xlibint.h:463:24:
| 463 | # define Xmalloc(size) malloc((size_t)(size))
| | ^~~~~~~~~~~~~~~~~~~~~~
| | |
| | (25) region created on heap here
XGetFCtl.c:152:20: note: in expansion of macro ‘Xmalloc’
| 152 | Feedback = Xmalloc(size);
| | ^~~~~~~
|
‘XGetFeedbackControl’: event 26
|
| 153 | if (!Feedback)
| | ^
| | |
| | (26) following ‘false’ branch (when ‘Feedback’ is non-NULL)...
|
‘XGetFeedbackControl’: event 27
|
|cc1:
| (27): ...to here
|
‘XGetFeedbackControl’: events 28-32
|
| 159 | for (i = 0; i < *num_feedbacks; i++) {
| | ~~^~~~~~~~~~~~~~~~
| | |
| | (28) following ‘true’ branch...
| 160 | switch (f->class) {
| | ~~~~~~ ~~~~~~~~
| | | |
| | | (29) ...to here
| | (30) following ‘default:’ branch...
|......
| 267 | f = (xFeedbackState *) ((char *)f + f->length);
| | ~~~~~~~~~
| | |
| | (31) ...to here
| 268 | Feedback = (XFeedbackState *) ((char *)Feedback + Feedback->length);
| | ~~~~~~~~~~~~~~~~
| | |
| | (32) use of uninitialized value ‘*Feedback.length’ here
|