Previously, if realloc failed to increase the size, we'd still
record that we had allocated the larger size, but the pointer
to it would be NULL, causing future calls to be broken, and the
previous allocation to be lost/leaked.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matthieu Herrb <matthieu.herrb@laas.fr>
Previous code seemed to assume that printf("%s", NULL) would result
in a 0-length string, not "(null)" or similar, but since there's no
point looking for files in "(null)/filepath...", instead we just
skip over NULL entries in search paths when generating file names.
In the *DirName() functions, this effectively just moves the "bail on
NULL in arg[i]" check up from the later code that assigned it to targetdir
and then bailed if that was NULL.
Not sure how there ever could be a NULL in arg[i], given the current
implementation of XlcParsePath, but it's easy enough to check once and
reject up front instead of on every reference.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
File Leak: Leaked File fp
at line 219 of lib/libX11/src/xlibi18n/XlcDL.c in function 'resolve_object'.
fp initialized at line 198 with fopen
[ This bug was found by the Parfait 1.2.0 bug checking tool.
http://labs.oracle.com/pls/apex/f?p=labs:49:::::P49_PROJECT_ID:13 ]
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matthieu Herrb <matthieu.herrb@laas.fr>
Commits 4076189869 and
f198c6aa98 added two new locales
(sr_CS.UTF-8 and km_KH.UTF-8), but didn't list them in configure.ac,
meaning they're not included in tarballs.
Signed-off-by: Julien Cristau <jcristau@debian.org>
Reviewed-by: James Cloos <cloos@jhcloos.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>
Upstreaming from changes originally integrated into OpenSolaris
under Sun bug id 6882572.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Thomas Klausner <wiz@NetBSD.org>
On the Xlib side, the only real difference is the mode flag we send
to the server with the address, so just make that an argument to the
function with the common code for packing the address into the request.
(Aside from labels, gcc 4.7.2 generates identical code before & after
this change due to inlining, verified via diff of gcc -S output.)
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Handle arbitrary length data in the same fashion as other calls,
avoiding need to ensure it fits all in the request buffer.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
This fixes the two callers of GetReqExtra to check that "req" is non-NULL
to avoid crashing now that GetReqExtra does internal bounds-checking on
the resulting buffer sizes.
Additionally updates comment describing return values to use names
instead of only literal values.
Signed-off-by: Kees Cook <kees@outflux.net>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Two users of GetReqExtra pass arbitrarily sized allocations from the
caller (ModMap and Host). Adjust _XGetRequest() (called by the GetReqExtra
macro) to double-check the requested length and invalidate "req" when
this happens. Users of GetReqExtra passing lengths greater than the Xlib
buffer size (normally 16K) must check "req" and fail gracefully instead
of crashing.
Any callers of GetReqExtra that do not check "req" for NULL
will experience this change, in the pathological case, as a NULL
dereference instead of a buffer overflow. This is an improvement, but
the documentation for GetReqExtra has been updated to reflect the need
to check the value of "req" after the call.
Bug that manifested the problem:
https://bugs.launchpad.net/ubuntu/+source/x11-xserver-utils/+bug/792628
Signed-off-by: Kees Cook <kees@outflux.net>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
[For all of these, LONG_MAX was the correct value to prevent overflows
for the recent CVEs. Lowering to INT_MAX catches buggy replies from
the server that 32-bit clients would reject but 64-bit would accept,
so we catch bugs sooner, and really, no sane & working server should
ever report more than 2gb of extension names, font path entries,
key modifier maps, etc. -alan- ]
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
clang complained (correctly):
warning: comparison of constant 768614336404564650 with expression
of type 'CARD32' (aka 'unsigned int') is always true
[-Wtautological-constant-out-of-range-compare]
[While LONG_MAX is correct, since it's used in size_t math, the
numbers have to be limited to 32-bit range to be usable by 32-bit
clients, and values beyond that range are far more likely to be
bugs in the data from the server than valid numbers of characters
in a font. -alan- ]
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Let libX11 load and make available the newer (X11R6) callback-based
API for XIM (expected by emacs).
This patch updates the files to match the other nls/ files.
Patch from Ian D. Leroux <idleroux@fastmail.fm> on pkgsrc-users@NetBSD.org
following a hint by Nhat Minh Lê <nhat.minh.le@gmail.com>.
Reviewed-by: James Cloos <cloos@jhcloos.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
It seems useless to do that since the code tests for both source
length and destination to be non-zero. This fixes a cut'n'paste
problem in xterm where the paste length was limited to 1024 (BUFSIZ)
in button.c.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
The XListPixmapFormats arguments was being shown with XImageByteOrder's
name and return types. Appears to have been a glitch in the nroff ->
docbook conversion.
Reported-by: ZHANG Zhaolong <zhangzl2013@126.com>
Reviewed-by: Jamey Sharp <jamey@minilop.net>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Avoid .TS H and .TH for now as it doesn't alter the output in this case,
and improve the output with mandoc(1).
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
The size of the arrays is max_key_code + 1. This makes these functions
consistent with the other checks added for CVE-2013-1997.
Also check the XkbGetNames reply when names->keys was just allocated.
Signed-off-by: Julien Cristau <jcristau@debian.org>
Tested-by: Colin Walters <walters@verbum.org>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Fixes builds with Solaris Studio 12.3 when lint is enabled, since it no
longer ignores *.h files, but complains when they reference undefined
typedefs or macros.
Signed-off-by: Niveditha Rau <Niveditha.Rau@Oracle.COM>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Freeing a pointer that wasn't returned by malloc() is undefined
behavior and produces an error with OpenBSD's implementation.
Signed-off-by: Matthieu Herrb <matthieu.herrb@laas.fr>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Try to offset the cost of all the recent checks we've added by giving
the compiler a hint that the branches that involve us eating data
are less likely to be used than the ones that process it.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Various other bounds checks in the code assume this is true, so
enforce it when we first get the data from the X server.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Prevents trying to free uninitialized pointers if we have to bail out
partway through setup, such as if we receive a corrupted or incomplete
connection setup block from the server.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Avoids memory corruption and other errors when callers access them
without checking to see if XGetWindowProperty() returned an error value.
Callers are still required to check for errors, this just reduces the
damage when they don't.
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>
Ensure that when breaking the returned list into individual strings,
we don't walk past the end of allocated memory to write the '\0' bytes
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matthieu Herrb <matthieu.herrb@laas.fr>
Ensure that when breaking the returned list into individual strings,
we don't walk past the end of allocated memory to write the '\0' bytes
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matthieu Herrb <matthieu.herrb@laas.fr>
Ensure that when breaking the returned list into individual strings,
we don't walk past the end of allocated memory to write the '\0' bytes
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matthieu Herrb <matthieu.herrb@laas.fr>
