Direct leak of 12 byte(s) in 2 object(s) allocated from:
#0 0x7f4f25c3f7a7 in strdup (/usr/lib64/libasan.so.6+0x5c7a7)
#1 0x7f4f252ce6a1 in _XimEncodeString libX11-1.8.3/modules/im/ximcp/imRm.c:818
#2 0x7f4f252ce6a1 in _XimEncodeString libX11-1.8.3/modules/im/ximcp/imRm.c:807
#3 0x7f4f252d2f0f in _XimSetICValueData libX11-1.8.3/modules/im/ximcp/imRm.c:2912
#4 0x7f4f252b536a in _XimLocalCreateIC libX11-1.8.3/modules/im/ximcp/imLcIc.c:176
#5
0x7f4f251f0105 in XCreateIC libX11-1.8.3/src/xlibi18n/ICWrap.c:251
detected and fix by Patrick Lerda <patrick9876@free.fr>
applied with adjustment, do changes when OOM (unlikely but good practise)
Analysis:
_XimRegisterIMInstantiateCallback() opens an XIM and closes it using
the internal function pointers, but the internal close function does
not free the pointer to the XIM (this would be done in XCloseIM()).
Report/patch:
Date: Mon, 03 Oct 2022 18:47:32 +0800
From: Po Lu <luangruo@yahoo.com>
To: xorg-devel@lists.x.org
Subject: Re: Yet another leak in Xlib
For reference, here's how I'm calling XRegisterIMInstantiateCallback:
XSetLocaleModifiers ("");
XRegisterIMInstantiateCallback (compositor.display,
XrmGetDatabase (compositor.display),
(char *) compositor.resource_name,
(char *) compositor.app_name,
IMInstantiateCallback, NULL);
and XMODIFIERS is:
@im=ibus
Signed-off-by: Thomas E. Dickey <dickey@invisible-island.net>
It is possible for _XimICOfXICID() to return NULL, so it is necessary
to check this isn't actually the case before dereferencing the pointer.
All other callers of _XimICOfXICID() do this check too.
(The check itself is ugly, but it follows the style of the code in the
rest of the module.)
Fixes issue #45.
Reported-by: Bhavi Dhingra
Original-patch-by: Bhavi Dhingra
Signed-off-by: Benno Schulenberg <bensberg@telfort.nl>
Adding the offset between the realloc result and the old allocation to
update pointers into the new allocation is undefined behaviour: the
old pointers are no longer valid after realloc() according to the C
standard. While this works on almost all architectures and compilers,
it causes problems on architectures that track pointer bounds (e.g.
CHERI or Arm's Morello): the value_list pointers will still have the
bounds of the previous allocation and therefore any dereference will
result in a run-time trap.
I found this due to a crash (dereferencing an invalid capability) while
trying to run `xev` over SSH on a CHERI-RISC-V system. With these two
realloc changes, and https://gitlab.freedesktop.org/xorg/proto/xorgproto/-/merge_requests/41
I am able to succesfully run `xev` compiled for CHERI-RISC-V.
Signed-off-by: Alex Richardson <Alexander.Richardson@cl.cam.ac.uk>
While these are mostly called during teardown of larger structures
that are about to themselves be freed, there's no guarantee that
will always be the case, so try to be safer here.
[ This bug was found by the Parfait 4.0 bug checking tool.
http://labs.oracle.com/pls/apex/f?p=labs:49:::::P49_PROJECT_ID:13 ]
v2: Deduplicate & simplify pointer clearing in _XFreeEventCookies
as suggested by @keithp
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Locale modifiers may be freed whenever XSetLocaleModifiers gets
called, even if the locale hasn't changed. This means that we cannot
save a pointer to those modifiers in the XimInstCallback record and
must, instead, make a copy of them instead.
This fixes a problem uncovered when running wish under libasan as
follows (on current Debian unstable):
$ LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libasan.so.6 wish
Reported-by: Vittorio Zecca <zeccav@gmail.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
v2:
Remove incorrect 'else' token found by @alanc
Avoids gcc warnings that we're using strncpy wrong to copy a known-length
set of characters without a terminating '\0' to a buffer whose length we
are checking separately. (Should also be imperceptibly faster since we
no longer check if each byte is '\0' when we already know it won't be.)
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
The check here guards the read below.
For `XimType_XIMStyles`, these are `num` of `CARD32` and for `XimType_XIMHotKeyTriggers`
these are `num` of `XIMTRIGGERKEY` ref[1] which is defined as 3 x `CARD32`.
(There are data after the `XIMTRIGGERKEY` according to the spec but they are not read by this
function and doesn't need to be checked.)
The old code here used the native datatype size instead of the wire protocol size causing
the check to always fail.
Also fix the size calculation for the header (size). It is 2 x CARD16 for both types
despite the unused `CARD16` for `XimType_XIMStyles`.
[1] https://www.x.org/releases/X11R7.6/doc/libX11/specs/XIM/xim.html#Input_Method_Styles
This fixes a regression caused by 388b303c62 in 1.6.10.
Fix#116
It's coming from a length in the protocol (unsigned) and passed
to functions that expect unsigned int parameters (_XCopyToArg()
and memcpy()).
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Reviewed-by: Todd Carson <toc@daybefore.net>
It looks like uninitialized stack or heap memory can leak
out via padding bytes.
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
The lengths are unsigned according to the specification. Passing
negative values can lead to data corruption.
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
cbb59d172 ('Braille: Fix typing quickly') broke the default lookup that
translates Braille keysym patterns to Braille Unicode patterns since it
rightfully clears brl_committing, but then we do not have it any more to
fill brl_committed.
This change saves the committed pattern so we can return it in the
default lookup.
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
On finishing releasing Braille keys, we should clear the just-commited
pattern, to reset the state to initial state, and avoid having to wait for
0.3s before typing the next pattern.
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Tested-by: Jean-Philippe Mengual <jpmengual@hypra.fr>
The _XimCacheStruct structure is followed in memory by two strings containing
fname and encoding. The memory was accessed using the last member of the
structure `char fname[1]`. That is a lie, prohibits us from using sizeof and
confuses checkers. Lets declare it properly as a flexible array, so compilers
don't complain about writing past that array. As bonus we can replace the
XOffsetOf with regular sizeof.
Fixes GCC8 error:
In function 'strcpy',
inlined from '_XimWriteCachedDefaultTree' at imLcIm.c:479:5,
inlined from '_XimCreateDefaultTree' at imLcIm.c:616:2,
inlined from '_XimLocalOpenIM' at imLcIm.c:700:5:
/usr/include/bits/string_fortified.h:90:10: error: '__builtin_strcpy'
forming offset 2 is out of the bounds [0, 1] [-Werror=array-bounds]
return __builtin___strcpy_chk (__dest, __src, __bos (__dest));
Caused by this line seemingly writing past the fname[1] array:
imLcIm.c:479: strcpy (m->fname+strlen(name)+1, encoding);
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
If we hit the depth limit, filename leaks. Move the depth check up before we
allocate filename.
Introduced in 226622349a.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
The Compose format has a feature which allows specifying certain
modifiers must (or must not) be present with a given keysym in the
sequence.
The grammar in imLcPrs.c and the Compose man page both do not match what
the code actually does (see the handling of the variables
`modifier_mask` and `modifier` in parseline() in imLcPrs.c, which are
eventually matched as `ev->state & modifier_mask == modifier`).
Also explicitly list the accepted modifier names, since they are
not standard (e.g. "Ctrl" instead of "Control").
Signed-off-by: Ran Benita <ran234@gmail.com>
Signed-off-by: James Cloos <cloos@jhcloos.com>
C89 or bust! This was documented as being needed for "only Lynx,
Linux-libc5, OS/2" and has never been enabled in modular builds,
since none of those platforms have had anyone step up to add support
since the X11R7 conversion to autotools.
Mostly performed with unifdef -UX_LOCALE, followed by removal of files
left without any purpose, and manual cleanup of remaining references.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Left one cast behind that is necessary to change from const char *
to char * in src/xlibi18n/lcCharSet.c.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
imRm.c: In function '_XimSetICMode':
imRm.c:2419:37: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
imRm.c:2420:30: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
lcGenConv.c: In function 'byteM_parse_codeset':
lcGenConv.c:345:13: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual]
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Fix bogus timestamp generted by XIM due to uninitialized
data field. Also set appropriate serial, too.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=39367
Signed-off-by: Chiaki ISHIKAWA <ishikawa@yk.rim.or.jp>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
XSetICFocus() and XUnsetICFocus() are both asynchronous events.
This is a pretty stupid idea: those functions may undo certain
settings on the client side for which requests from the server
may still be in the queue unprocessed. Thus things may be set
in the wrong order ie instead of set -> unest it will be unset -> set.
Moreover there is no way for either the client or the server to
cause the event queue to be flushed - which is pretty bad as
XIM is bidirectional.
The scenario is as follows:
Two ICs are created:
ic1 = XCreateIC(im,
XNInputStyle, XIMPreeditCallbacks | XIMStatusCallbacks,
XNClientWindow, window,
XNPreeditAttributes, preedit_attr,
XNStatusAttributes, status_attr,
NULL);
ic2 = XCreateIC(im, XNInputStyle,
XIMPreeditNothing | XIMStatusNothing,
XNClientWindow, window, NULL);
Then the focus is removed from ic2:
XUnsetICFocus(ic2);
If SCIM is used as the input server it will send a bunch of requests
following an XCreateIC(). One of the requests registers a key release
filter. XUnsetICFocus() unsets both key press and release filters.
Since it is asynchronous, the input server requests to register key
press and release filters may not have been processed, when XUnsetICFocus()
is called. Since there is no explicite way for client programs to enforce
the request queue to be flushed explicitely before an X[Set/Unset]ICFocus()
call it would be safest to make those two calls synchronous in the sense
that they ensure the request queue has been handled before they execute.
The easiest way to do this from Xlib is thru a call to XGetICValues()
which sends a request to the server and subsequently reads the queue
from the server to the client. This will cause all outstanding requests
in the queue to be read and handled.
This is an ugly hack and this could be fixed directly in the client,
however it seems to be easier to fix Xlib than to fix numerous clients.
This problem arose since there is no well documented way how to handle
and synchronize XIM requests and not all input servers send requests
when an IC is created.
This has been discussed extensively in:
https://bugzilla.novell.com/show_bug.cgi?id=221326
Signed-off-by: Egbert Eich <eich@freedesktop.org>
When synthesized key events are sent on commit XIM sets the 'fabricated'
flag so that the keypress handler knows that these were not real events.
This also happens when committing due to the loss of focus. However in this
case the keypress/release filters which consume and unset this flag are no
longer in the filter chain.
So the flag is erronously set when a real keyboard event is received after
focus has been regained. So the first event is wrongly treated as a
fabricated key in the keypress handler which will at the same time reset
the flag so the second key event is treated correctly.
This fix only sets the flag when at least one of the keyboard filters is in
place.
How to reproduce this bug: run scim, choose a Japanese input method start
two instances of xterm: start typing in one xterm (this should pop up an
IM window). Without comitting (hitting 'enter') move focus to the other
xterm, then move focus back. Start typing again. The first character will
be committed immediately without popping up an input window.
With this fix this behavior is gone.
See also: https://bugzilla.novell.com/show_bug.cgi?id=239698
Signed-off-by: Egbert Eich <eich@freedesktop.org>
When trying to process file paths the tokens %H, %L, & %S are expanded
to $HOME, the standard compose file path & the xlocaledir path.
If enough of these tokens are repeated and values like $HOME are set to
very large values, the calculation of the total string size required to
hold the expanded path can overflow, resulting in allocating a smaller
string than the amount of data we'll write to it.
Simply restrict all of these values, and the total path size to PATH_MAX,
because really, that's all you should need for a filename path.
Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matthieu Herrb <matthieu.herrb@laas.fr>
Called from _XimCreateDefaultTree() which uses getenv("XCOMPOSEFILE")
to specify filename.
If the size of off_t is larger than the size of unsigned long (as in
32-bit builds with large file flags), a file larger than 4 gigs could
have its size truncated, leading to data from that file being written
past the end of the undersized buffer allocated for it.
While configure.ac does not use AC_SYS_LARGEFILE to set large file mode,
builders may have added the large file compilation flags to CFLAGS on
their own.
size is left limited to an int, because if your Xim file is
larger than 2gb, you're doing it wrong.
Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matthieu Herrb <matthieu.herrb@laas.fr>
