[jira] [Updated] (GUACAMOLE-2342) guacd segfaults on every RDP connection with FreeRDP 3.32.0 (>=3.6.0): rdp_freerdp_pre_connect() calls gdi_init() which dereferences context->cache, but FreeRDP no longer provides any public API to allocate rdpCache

["Nick Couchman (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/GUACAMOLE-2342?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Nick Couchman updated GUACAMOLE-2342:
-------------------------------------
    Priority: Major  (was: Blocker)

> guacd segfaults on every RDP connection with FreeRDP 3.32.0 (>=3.6.0): rdp_freerdp_pre_connect() calls gdi_init() which dereferences context->cache, but FreeRDP no longer provides any public API to allocate rdpCache
> -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GUACAMOLE-2342
>                 URL: https://issues.apache.org/jira/browse/GUACAMOLE-2342
>             Project: Guacamole
>          Issue Type: Bug
>          Components: guacamole-server, RDP
>    Affects Versions: 1.6.0
>            Reporter: Wuschy
>            Priority: Major
>
> Summary
>  
>   Building guacamole-server 1.6.0 natively against a current FreeRDP3 (3.32.0, Ubuntu 26.04 packages) produces a guacd that segfaults inside FreeRDP's own gdi_init_ex() on every single RDP connection attempt, immediately after logging
>   the local/remote framebuffer pixel formats, before any RDP capability negotiation with the target server takes place.
>  
>   Root causes found (two real, one structural gap)
>  
>   1. configure.ac's FreeRDP-context detection test is poisoned by -Werror
>  
>   Before the "whether freerdp structs have a context" AC_COMPILE_IFELSE test runs (configure.ac, ~line 1023), CPPFLAGS already contains -Werror from an earlier RDP_CFLAGS check (configure.ac lines ~744/756). FreeRDP3's own public headers
>   (winpr/sspi.h, freerdp/codecs.h, freerdp/freerdp.h) emit several -Wdeprecated-declarations warnings when merely included (unrelated to anything the test program actually uses). Under -Werror, the test program fails to compile for a
>   reason that has nothing to do with whether instance->context->input exists, so FREERDP_HAS_CONTEXT is left undefined even though the installed FreeRDP clearly has a context (3.32.0 obviously does). This silently selects the wrong
>   GUAC_RDP_CONTEXT() macro branch in src/protocols/rdp/rdp.h:
>  
>   #ifdef FREERDP_HAS_CONTEXT
>   #define GUAC_RDP_CONTEXT(rdp_instance) ((rdp_instance)->context)
>   #else
>   #define GUAC_RDP_CONTEXT(rdp_instance) ((rdp_instance))
>   #endif
>   
>   With the macro resolving to the second branch, every GUAC_RDP_CONTEXT(rdp_inst)->input access in input-queue.c/keyboard.c becomes a direct ((struct rdp_freerdp*) rdp_inst)->input access, and rdp_freerdp has no input member any more
>   unless built WITH_FREERDP_DEPRECATED (see freerdp.h: WINPR_DEPRECATED_VAR("use rdpContext::input instead", ALIGN64 rdpInput* input;) guarded by #if defined(WITH_FREERDP_DEPRECATED)). Hence the build fails with:
>  
>   input-queue.c:134:43: error: 'freerdp' \{aka 'struct rdp_freerdp'} has no member named 'input'
>   Workaround: configure with CPPFLAGS=-Wno-deprecated-declarations ./configure ... — this alone fixes FREERDP_HAS_CONTEXT detection and the build error above. This should arguably be fixed in configure.ac itself (either don't poison
>   CPPFLAGS with -Werror before this test, or add -Wno-deprecated-declarations to the test program's own CFLAGS).
>  
>   2. context->codecs is never allocated by guacamole-server
>  
>   Since FreeRDP 3.6.0, context->codecs (rdpCodecs*) is no longer automatically allocated. The current freerdp/codecs.h documents the replacement API explicitly:
>  
>   FREERDP_API rdpCodecs* freerdp_client_codecs_new(UINT32 ThreadingFlags);
>   FREERDP_API void freerdp_client_codecs_free(rdpCodecs* codecs);
>   guacamole-server's src/protocols/rdp/rdp.c never calls either the old deprecated codecs_new(rdpContext*) or the new freerdp_client_codecs_new() anywhere. context->codecs is therefore NULL when gdi_init() is called from
>   rdp_freerdp_pre_connect() (rdp.c:196), and FreeRDP's internal gdi_init_ex() dereferences it.
> Fix applied locally (rdp.c, immediately before the existing gdi_init() call):
>  
>   /* FreeRDP >= 3.6.0 no longer auto-allocates context->codecs; gdi_init()
>    * dereferences it internally, so it must be created explicitly here or
>    * guacd segfaults in gdi_init_ex() on every connection attempt. */
>   context->codecs = freerdp_client_codecs_new(0);
>   if (context->codecs == NULL)
>       return FALSE;
>       
>   This fix is correct and necessary, confirmed via core dump: after applying it, context->codecs is no longer NULL (verified with gdb against a core dump of the crash).
>   
>   3. context->cache is NULL and FreeRDP3 provides no public way to allocate it (blocking)
>   After fixing #1 and #2, guacd still segfaults at the exact same instruction inside gdi_init_ex() (confirmed via gdb: #0 0x... in gdi_init_ex () from libfreerdp3.so.3, called from rdp_freerdp_pre_connect (rdp.c:204) → freerdp_connect()
>   → guac_rdp_handle_connection (rdp.c:597)).
>   Inspecting the core dump with full debug symbols on guacamole-server's own frame:
>   (gdb) print context->cache
>   $2 = (rdpCache *) 0x0
>   (gdb) print context->codecs
>   $3 = (rdpCodecs *) 0x... (non-NULL, confirms fix #2 worked)
>   
>   context->cache (rdpCache*, rdpContext offset ~35, "/* 35 */" in freerdp.h's struct comments) is NULL, and the crashing instruction dereferences it (mov 0x8(%rdx),%rdx with %rdx == 0, si_addr = 0x8 per $_siginfo).
>  
>   guacamole-server's src/protocols/rdp/*.c never references rdpCache, context->cache, or any cache constructor anywhere — confirmed via grep -rn 'cache_new\|rdpCache\|context->cache' src/protocols/rdp/ (zero matches). This presumably
>   used to be populated automatically by an earlier FreeRDP version's internals.
>   The problem: as of FreeRDP 3.32.0, there is no public API to allocate an rdpCache* at all:
>   - /usr/include/freerdp3/freerdp/cache/ contains only persistent.h (unrelated: disk-based persistent bitmap cache). The old combined cache.h (bitmap/glyph/brush/offscreen/palette sub-caches + cache_new()) does not exist in the installed
>     headers.
>   - nm -D libfreerdp3.so.3 | grep -i cache_new → no match at all (only persistent_cache_new, unrelated).
>   So either:
>   (a) gdi_init_ex() has legacy code referencing context->cache that should have been removed/guarded when the public cache.h API was retired, and this is a genuine FreeRDP regression, or
>   (b) guacamole-server is expected to switch its RDP client bootstrap from the "raw" freerdp_new() + manual rdpContext setup it currently uses, to the higher-level freerdp_client_context_new() convenience wrapper (used by the official
>   xfreerdp3 client), which may populate context->cache via a non-exported internal path that raw freerdp_new() skips. This would be a substantially bigger refactor of guacd's RDP connection bootstrap, not a one-line fix.
>   Tested and ruled out: setting disable-bitmap-caching=true and disable-offscreen-caching=true as Guacamole RDP connection parameters does NOT avoid the crash — it is unconditional, independent of these settings.
> Reproduction
>   - Ubuntu 26.04 LTS, libfreerdp3-3 / freerdp3-dev 3.32.0+dfsg-0ubuntu0.26.04.1
>   - guacamole-server 1.6.0 (git tag), built with:
>   autoreconf -fi
>   CPPFLAGS=-Wno-deprecated-declarations ./configure --with-init-dir=/etc/init.d
>   make -j4 CFLAGS='-g -O2 -Wno-error' CXXFLAGS='-g -O2 -Wno-error'
>     (with fix #2 applied to rdp.c as above)
>   - Any RDP connection (tested against both a plain xrdp/X11 target and a GNOME gnome-remote-desktop EGFX target — both crash identically, so this is unrelated to the target server, purely a guacd/FreeRDP3-client-side issue)
>   - 100% reproducible, every attempt, no exceptions observed
>  
>   Suggested fix direction
>  
>   Either:
>   1. FreeRDP exposes a minimal public constructor for rdpCache again (even a stub that satisfies whatever gdi_init_ex() needs), or
>   2. gdi_init_ex() is patched to tolerate context->cache == NULL given the legacy bitmap/glyph/brush cache subsystem it represents has otherwise been fully retired from the public API, or
>   3. guacamole-server's RDP client is refactored to bootstrap via freerdp_client_context_new() instead of raw freerdp_new().
>   Happy to provide the full core dump / gdb session or test further fixes against our build if useful.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)