GUACAMOLE-2262: Heap-allocate clipboard buffers to prevent stack overflow.#659
Open
escra wants to merge 1 commit intoapache:staging/1.6.1from
Open
GUACAMOLE-2262: Heap-allocate clipboard buffers to prevent stack overflow.#659escra wants to merge 1 commit intoapache:staging/1.6.1from
escra wants to merge 1 commit intoapache:staging/1.6.1from
Conversation
…flow. The clipboard receive and send buffers in cliprdr.c were allocated on the stack using GUAC_COMMON_CLIPBOARD_MAX_LENGTH. Since configurable clipboard limits (GUACAMOLE-2002) raised this to 50 MiB while the default thread stack is only 8 MiB, large clipboard operations cause a stack overflow (SIGSEGV) that silently kills the connection. This change heap-allocates both buffers using guac_mem_alloc() and frees them on all exit paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The clipboard receive and send buffers in
cliprdr.cwere allocated on thestack using
GUAC_COMMON_CLIPBOARD_MAX_LENGTH. Since configurable clipboardlimits (GUACAMOLE-2002)
raised this to 50 MiB while the default thread stack is only 8 MiB, large
clipboard operations cause a stack overflow (SIGSEGV) that silently kills the
connection.
Root cause
guac_rdp_cliprdr_format_data_response()declares:c char received_data[GUAC_COMMON_CLIPBOARD_MAX_LENGTH];With
GUAC_COMMON_CLIPBOARD_MAX_LENGTHat 50 MiB (52428800 bytes) and thedefault pthread stack size at 8 MiB, this overflows the stack immediately.
Similarly,
guac_rdp_cliprdr_format_data_request()heap-allocates the outputbuffer using
guac_mem_alloc(GUAC_COMMON_CLIPBOARD_MAX_LENGTH)but could usethe actual configured clipboard size instead.
Fix
guac_mem_alloc()with the actual configuredclipboard size (
clipboard->clipboard->available) instead of the compile-timemaximum
formats)
Affected versions
staging/1.6.1andmain- any build with configurable clipboard limits(merged via PR GUACAMOLE-2002: Allow connection clipboard limits to be configured. #564)
JIRA
GUACAMOLE-2002 - this is
a regression from the configurable clipboard limits feature.
Test plan