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: xorg/lib/libice#4

Reported-by: mahendra <mahendra.n@samsung.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Allison Lortie authored Jun 14, 2016 and Alan Coopersmith committed Mar 24, 2019

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>

Allison Lortie authored Jun 14, 2016 and Alan Coopersmith committed Mar 24, 2019

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>

Tobias Stoeckmann authored Jul 30, 2018 and Alan Coopersmith committed Mar 10, 2019

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>

Walter Harms authored Sep 07, 2017 and Alan Coopersmith committed Mar 03, 2019

commit ff5e59f3

 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>

Walter Harms authored Sep 08, 2017 and Emil Velikov committed Sep 14, 2017

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>

Walter Harms authored Sep 08, 2017 and Emil Velikov committed Sep 14, 2017

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>

Eric Engestrom authored Jul 07, 2017 and Emil Velikov committed Sep 07, 2017

`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>

Eric Engestrom authored Jul 07, 2017 and Emil Velikov committed Sep 07, 2017

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>

Eric Engestrom authored Jul 07, 2017 and Emil Velikov committed Sep 07, 2017

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 72e35356


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)

Benjamin Tissoires authored Apr 04, 2017 and Peter Hutterer committed Apr 24, 2017

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>

Mihail Konev authored Jan 26, 2017 and Peter Hutterer committed Jan 26, 2017

Signed-off-by: Mihail Konev <k.mvc@ya.ru>

Emil Velikov authored Mar 09, 2015 and Peter Hutterer committed Jan 26, 2017

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>

Tobias Stoeckmann authored Nov 22, 2016 and Matthieu Herrb committed Dec 09, 2016

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>

Remko van der Vossen authored Jul 19, 2015 and Alan Coopersmith committed Jul 19, 2015

https://bugs.freedesktop.org/show_bug.cgi?id=90616



Recent versions of gcc have array bounds checking turned on by default,
this leads to build failures of libICE. As the _IceVersionCount variable
in ICElibint.h is not declared const the compiler cannot assume that the
nested for loop in ProcessConnectionSetup stays within bounds.

The simple fix is of course to change the declarations of _IceVersionCount,
_IceVersions, and the local variable myVersionCount to const declarations.
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
Reviewed-by: David Macek <david.macek.0@gmail.com>

Reported-by: Jasper St. Pierre <jstpierre@mecheye.net>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Jasper St. Pierre <jstpierre@mecheye.net>

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Found by cppcheck 1.62:
[src/register.c:84]: (style) Variable 'name' is assigned a value
 that is never used.
[src/register.c:182]: (style) Variable 'name' is assigned a value
 that is never used.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Found by cppcheck 1.62:
[src/accept.c:113]: (error) Memory leak: iceConn.connection_string
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

arc4random() and associated functions can be found in libbsd on
GNU/Linux systems.
Signed-off-by: Matthieu Herrb <matthieu.herrb@laas.fr>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Clears a number of clang warnings of the form:

connect.c:328:6: warning: implicit conversion loses integer precision:
   'size_t' (aka 'unsigned long') to 'CARD16' (aka 'unsigned short')
   [-Wconversion]
            STORE_STRING (pData, _IceAuthNames[i]);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./ICElibint.h:173:19: note: expanded from macro 'STORE_STRING'
    CARD16 _len = strlen (_string); \
           ~~~~   ^~~~~~~~~~~~~~~~
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Clears many clang warnings about implicit conversions losing
integer precision, such as from storing ints into CARD8 or CARD16.
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>