| Age | Commit message (Collapse) | Author |
|---|
|
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
The only files libICE operates on are .ICEauthority files, which
it only uses internally and does not make available to other code,
so there is no concern about ABI mismatch here.
While .ICEauthority files should never be more than 2gb in size,
they may be stored on filesystems with large inodes.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Fixes: b9411f7 ("ice.pc.in: add -lbsd flags when required")
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Some implementations of static_assert() define a new variable.
Avoid warnings from those when calling static_assert() from a
macro that may not be at the top of a new code block.
../../src/accept.c: In function 'IceAcceptConnection':
../../src/accept.c:159:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
159 | IceGetHeader (iceConn, 0, ICE_ByteOrder,
| ^~~~~~~~~~~~
../../src/connect.c: In function 'IceOpenConnection':
../../src/connect.c:254:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
254 | IceGetHeader (iceConn, 0, ICE_ByteOrder,
| ^~~~~~~~~~~~
../../src/connect.c:340:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
340 | IceGetHeaderExtra (iceConn, 0, ICE_ConnectionSetup,
| ^~~~~~~~~~~~~~~~~
[...etc...]
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Avoid unnecessary library dependency when using a libc with these
functions included
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
This is the preferred usage form for libbsd, as it makes the code more
portable and requires no special includes for libbsd, by transparently
injects the needed standard headers that would be used on a BSD.
Signed-off-by: Guillem Jover <guillem@hadrons.org>
|
|
Fixes numerous gcc warnings of the form:
connect.c: In function ‘IceOpenConnection’:
ICElibint.h:160:25: warning: potential null pointer dereference [-Wnull-dereference]
*((CARD16 *) _pBuf) = _val; \
^
ICElibint.h:174:5: note: in expansion of macro ‘STORE_CARD16’
STORE_CARD16 (_pBuf, _len); \
^~~~~~~~~~~~
connect.c:351:5: note: in expansion of macro ‘STORE_STRING’
STORE_STRING (pData, IceReleaseString);
^~~~~~~~~~~~
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
A message header length larger than ICE_OUTBUFSIZE will cause
buffer overflows.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
It should never happen, but has been possible in the past when
we didn't handle buffer checks properly - this would help us
catch it if a similar mistake ever happens again (or the wrong
memory pointer gets corrupted by something else).
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Reduce duplicated code in _IceRead() and _IceWrite()
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
If there's not room for it in the buffer, we already set pData to
NULL, but still set the outbufptr to include the space, which could
lead to IceFlush() reading past the end of the buffer.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Signed-off-by: Walter Harms <wharms@bfs.de>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
fix a potential null pointer deference error
Signed-off-by: Walter Harms <wharms@bfs.de>
|
|
In function 'IceOpenConnection': gcc give the following warning:
connect.c:106:11: warning: declaration of 'iceConn' shadows a previous local [-Wshadow]
fixed by renaming 2. iceConn to iConn (and all its uses)
Signed-off-by: Walter Harms <wharms@bfs.de>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Fixes gcc complaint:
process.c: In function ‘ProcessAuthReply’:
process.c:1478:20: warning: declaration of ‘status’ shadows a previous local [-Wshadow]
1478 | Status status = 1;
| ^~~~~~
process.c:1426:25: note: shadowed declaration is here
1426 | IcePaAuthStatus status =
| ^~~~~~
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
This resolves an issue reported by the Oracle Parfait static analyzer:
Error: Use after free
Use after free [use-after-free] (CWE 416):
Use after free of pointer trans_conn
at line 566 of lib/libICE/src/connect.c in function 'ConnectToPeer'.
trans_conn previously freed with _IceTransClose at line 532
trans_conn was allocated at line 525 with _IceTransOpenCOTSClient
even though I believe this is already handled by the
'if (madeConnection) { ... } else trans_conn = NULL;'
block, but the analyzer apparently doesn't follow that logic,
while this simple change makes it obvious.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
If we're going to link to libbsd, might as well use it for strlcpy too
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Found by using:
codespell --builtin clear,rare,usage,informal,code,names
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
|
|
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Assigning and testing a value in a single statement hinders code clarity
and may confuses static code analyzers.
Separate the assignment and the test for clarity.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
|
|
`_IceRead()` gets called from multiple places which do not expect the
connection to be freed.
Do not free the connection data in `_IceRead()` to avoid potential
use-after-free issue in the various callers.
The connection data will be freed eventually in `ProcessWantToClose()`,
so not freeing it in `_IceRead()` should not introduce an memory leak.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
|
|
The function `_IceTransMakeAllCOTSServerListeners` allocates memory for
`transConns` which is leaked in case of error.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
|
|
Previously it would just bump the pointer in the buffer leaving
whatever values were previously there in place.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Fixes: https://gitlab.freedesktop.org/xorg/lib/libice/issues/4
Reported-by: mahendra <mahendra.n@samsung.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
If we find that $XDG_RUNTIME_DIR is set (and $ICEAUTHORITY is not), then
the ICEauthority file is stored in the XDG_RUNTIME_DIR instead of the
home directory, and without a leading dot.
https://bugs.freedesktop.org/show_bug.cgi?id=49173
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
There is a theoretical edge case where the $HOME environment variable
could be set to the empty string. IceAuthFileName() unconditionally
checks index 1 of this string, which is out of bounds.
Fix that up by rejecting empty strings in the same way as we reject
NULL.
https://bugs.freedesktop.org/show_bug.cgi?id=49173
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
The function strncpy does not guarantee to append a terminating
NUL character to the destination.
This patch merges libSM's way of handling this issue into libICE.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
commit ff5e59f32255913bb1cdf51441b98c9107ae165b left ret outside the #if
causing a gcc warning:
In function 'arc4random_buf':
iceauth.c:89:13: warning: unused variable 'ret' [-Wunused-variable]
fixed by moving #if 1 up
Signed-off-by: Walter Harms <wharms@bfs.de>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
|
|
I found IceProtocolShutdown() hard to read only to find that was
it does it aktually very simple. So i rearranged the code to make
it more readable.
Signed-off-by: Walter Harms <wharms@bfs.de>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
[Emil Velikov: whitespace fixes]
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
|
|
free() can handle NULL just fine - remove the check.
Signed-off-by: Walter Harms <wharms@bfs.de>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
|
|
`error_message` is passed in to strncpy() without any check, which
doesn't handle NULL itself, so let's make it a valid empty string in
cases where it was NULL.
Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
Acked-by: Walter Harms <wharms@bfs.de>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
|
|
Similar to the previous commit, assigning a static string would crash
upon freeing.
Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
Acked-by: Walter Harms <wharms@bfs.de>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
|
|
If the `errorClass` isn't handled by the switch, `errorStr`'s initial
value would be a pointer to some static memory with an empty string,
and freeing it would most likely crash.
Let's set it to NULL instead, as is done in other similar places.
Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
Acked-by: Walter Harms <wharms@bfs.de>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
|
|
Similar to ICE_t just set the define globally and remove the multiple
definitions throughout the tree
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com> (IRC)
|
|
Already defined at global scale in configure.ac
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com> (IRC)
|
|
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com> (IRC)
|
|
Analogous to previous commit, including the megacommit that removed the
need for it.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com> (IRC)
|
|
Directly use the strstr function as opposed to wrapping it in a macro.
The latter is no longer needed as of
commit 72e353567f8927996a26e72848d86f692c3f0737
Author: Kaleb Keithley <kaleb@freedesktop.org>
Date: Fri Nov 14 16:48:46 2003 +0000
XFree86 4.3.0.1
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com> (IRC)
|
|
This allows to fix CVE-2017-2626 on Linux platforms without pulling in
libbsd.
The libc getentropy() is available since glibc 2.25 but also on OpenBSD.
For Linux, we need at least a v3.17 kernel. If the recommended
arc4random_buf() function is not available, emulate it by first trying
to use getentropy() on a supported glibc and kernel. If the call fails,
fall back to the current (partly vulnerable) code.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
|
|
Signed-off-by: Mihail Konev <k.mvc@ya.ru>
|
|
Place quotes around the $srcdir, $ORIGDIR and $0 variables to prevent
fall-outs, when they contain space.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
|
|
Syncs the invocation of configure with the one from the server.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
|
|
The function IceAuthFileName is vulnerable to a use after free. The
flaw can be triggered by calling the function three times:
- First call succeeds and stores the path in buf, a dynamically
allocated buffer with size bsize.
- Second call fails due to out of memory. It frees buf, but keeps
the old size in bsize.
- Third call only checks if bsize is large enough. Then it uses
buf without allocating it again -- the use after free happens.
In order to exploit this, an attacker must change environment variables
between each call, namely ICEAUTHORITY or HOME. It also takes subsequent
calls. Due to these limitations, I don't consider this to be of high
priority.
Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
|
