summaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2022-09-14Handle arrays too large to fit in iceConn buffersAlan Coopersmith
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>
2022-09-07IceFlush: signal fatal I/O error if bufptr is past end of bufferAlan Coopersmith
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>
2022-09-07Refactor Fatal I/O error handling into a common functionAlan Coopersmith
Reduce duplicated code in _IceRead() and _IceWrite() Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2022-08-20make sure buffer is zero filled and report if allocation failedwalter harms
Signed-off-by: Walter Harms <wharms@bfs.de> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2022-08-20add check for mallocwalter harms
fix a potential null pointer deference error Signed-off-by: Walter Harms <wharms@bfs.de>
2022-04-30connect.c: FIX 'iceConn' shadows a previous local, [-Wshadow]walter harms
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>
2022-04-30ProcessAuthReply: rename status variable to avoid shadowingAlan Coopersmith
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>
2022-04-30ConnectToPeer: be doubly sure that use-after-free doesn't happenAlan Coopersmith
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>
2022-04-30Fix spelling/wording issuesAlan Coopersmith
Found by using: codespell --builtin clear,rare,usage,informal,code,names Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2019-04-11cleanup: Separate variable assignment and testOlivier Fourdan
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>
2019-04-10_IceRead: Avoid possible use-after-freeOlivier Fourdan
`_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>
2019-04-10IceListenForWellKnownConnections: Fix memleakOlivier Fourdan
The function `_IceTransMakeAllCOTSServerListeners` allocates memory for `transConns` which is leaked in case of error. Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
2019-03-24IceOpenConnection: check for malloc failure on connect_to_you tooAlan Coopersmith
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>
2019-03-24authutil: support $XDG_RUNTIME_DIR/ICEauthorityAllison Lortie
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>
2019-03-24authutil: fix an out-of-bounds accessAllison Lortie
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>
2019-03-10Always terminate strncpy results.Tobias Stoeckmann
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>
2019-03-03iceauth.c: FIX warning: unused variable 'ret' in 'arc4random_buf'walter harms
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>
2017-09-14make IceProtocolShutdown() more readablewalter harms
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>
2017-09-14Drop NULL check prior to free()walter harms
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>
2017-09-07Make sure string is never NULLEric Engestrom
`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>
2017-09-07Make sure error_message is a free-able stringEric Engestrom
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>
2017-09-07Make sure errorStr is a free-able stringEric Engestrom
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>
2017-09-07configure.ac: set TRANS_CLIENT/SERVEREmil Velikov
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)
2017-09-07Kill off local ICE_t definitionsEmil Velikov
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)
2017-09-07Remove unneeded ^L symbols.Emil Velikov
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)
2017-09-07Kill off Time_t macroEmil Velikov
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)
2017-09-07Kill off Strstr macroEmil Velikov
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)
2017-04-24Use getentropy() if arc4random_buf() is not availableBenjamin Tissoires
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>
2016-12-09Fix use after free on subsequent callsTobias Stoeckmann
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>
2015-07-19Bug 90616 - libICE build fails on array bounds checkRemko van der Vossen
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>
2015-04-10Include unistd.h for getpid()Jon TURNEY
Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk> Reviewed-by: David Macek <david.macek.0@gmail.com>
2013-12-24Delete unused name variable in register.cAlan Coopersmith
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>
2013-12-24Free iceConn->connection_string when unwinding after malloc failsAlan Coopersmith
Found by cppcheck 1.62: [src/accept.c:113]: (error) Memory leak: iceConn.connection_string Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2013-09-12Use arc4random when available to produce the auth cookie.Matthieu Herrb
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>
2013-08-08Make STORE_STRING cast strlen result to CARD16 when storing in CARD16Alan Coopersmith
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>
2013-08-08Make write_string call write_counted_string instead of copying itAlan Coopersmith
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2013-08-08Mark input arguments to write_string functions as constAlan Coopersmith
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2013-08-08Stop casting return values from mallocAlan Coopersmith
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2013-08-08Get rid of casts to (char *) in calls to free()Alan Coopersmith
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2013-07-20Convert remaining sprintf() call to snprintf()Alan Coopersmith
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2013-07-20Convert strcpy/strcat pairs to snprintf callsAlan Coopersmith
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2013-07-20Fix some clang warnings about integer sign/size conversionsAlan Coopersmith
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2013-07-20unifdef WORD64Alan Coopersmith
Remove leftover remnants of CRAY support, which still had some functions consisting solely of /* NOT IMPLEMENTED YET */ comments. Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2013-07-20Constify filename argument to IceLockAuthFile & IceUnlockAuthFileAlan Coopersmith
Needed to fix const string warnings in iceauth - functions already copy provided arguments to temporary local buffer for modifications. Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
2013-01-04unifdef -U__UNIXOS2__Alan Coopersmith
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
2011-11-30Plug minor memory leak in unusual path through ProcessConnectionSetupAlan Coopersmith
Error: Memory leak (CWE 401) Memory leak of pointer 'release' allocated with malloc((_len + 1)) at line 1100 of src/process.c in function 'ProcessConnectionSetup'. 'release' allocated at line 920 with malloc((_len + 1)). release leaks when _i >= hisAuthCount at line 925 and i >= _IceAuthCount at line 973 and found != 0 at line 998 and status != 0 at line 1053 and status != 1 at line 1070 and accept_setup_now == 0 at line 1082 and i >= hisAuthCount at line 1093. Memory leak of pointer 'vendor' allocated with malloc((_len + 1)) at line 1100 of src/process.c in function 'ProcessConnectionSetup'. 'vendor' allocated at line 919 with malloc((_len + 1)). vendor leaks when _i >= hisAuthCount at line 925 and i >= _IceAuthCount at line 973 and found != 0 at line 998 and status != 0 at line 1053 and status != 1 at line 1070 and accept_setup_now == 0 at line 1082 and i >= hisAuthCount at line 1093. [ This bug was found by the Parfait 0.3.7 bug checking tool. For more information see http://labs.oracle.com/projects/parfait/ ] Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Jeremy Huddleston <jeremyhu@apple.com>
2011-11-22Constify protocol, vendor & release string args to IceRegisterForProtocol*Alan Coopersmith
Needed to resolve gcc -Wwrite-strings warnings in callers. These functions only pass the strings to strcmp before calling strdup to make their own private copy for storing away. While fixing the API docs to match, also fix them to match the existing function prototypes, where there were several errors before (including just plain missing most of the args to IceRegisterForProtocolReply). Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Jeremy Huddleston <jeremyhu@apple.com>
2011-11-11Remove ancient workaround for System V/386 Release 4.2 compiler bugAlan Coopersmith
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Matthieu Herrb <matthieu.herrb@laas.fr> Reviewed-by: walter <wharms@bfs.de> Reviewed-by: Jeremy Huddleston <jeremyhu@apple.com>
2011-11-11Fix gcc -Wwrite-strings warnings in process.cAlan Coopersmith
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Jeremy Huddleston <jeremyhu@apple.com>
2011-11-11Fix gcc -Wwrite-strings warnings in AuthNames handlingAlan Coopersmith
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Jeremy Huddleston <jeremyhu@apple.com>
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-23 00:34:38 +0000