Backport CVE-2023-39354.

This commit is contained in:
Tobias Frost 2023-10-07 12:07:10 +02:00
parent 2dacc519e5
commit ab18013d96
4 changed files with 303 additions and 22 deletions

1
debian/changelog vendored
View File

@ -15,6 +15,7 @@ freerdp2 (2.3.0+dfsg1-2~deb10u3) UNRELEASED; urgency=medium
CVE-2020-15103 (Closes: #965979)
* Backporting/Importing upstream patches for (Closes: #1051638):
CVE-2023-39350 CVE-2023-39351 CVE-2023-39352 CVE-2023-39353
CVE-2023-39354
-- Tobias Frost <tobi@debian.org> Mon, 02 Oct 2023 17:10:48 +0200

View File

@ -0,0 +1,264 @@
Description: Upstream fix for CVE-2023-39354 - Out-Of-Bounds Read in nsc_rle_decompress_data
commit 1 of 2.
Backported changes required, e.g Stream_CheckAndLogRequiredLengthWLogEx() and WPR_ASSERT substituted.
with aseert()
Origin: https://github.com/FreeRDP/FreeRDP/commit/82ac0164f330c08ddd9a6ef6f3dbf846c4b79def
Bug: https://github.com/FreeRDP/FreeRDP/security/advisories/GHSA-c3r2-pxxp-f8r6
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1051638
From 82ac0164f330c08ddd9a6ef6f3dbf846c4b79def Mon Sep 17 00:00:00 2001
From: Armin Novak <anovak@thincast.com>
Date: Sat, 5 Aug 2023 10:53:29 +0200
Subject: [PATCH] [codec,nsc] fix missing plane length check
reported by @pwn2carr
(cherry picked from commit 89cd622426b3fcac64b9203e69f9f407cc19760e)
---
libfreerdp/codec/nsc.c | 71 +++++++++++++++++++++---------------------
1 file changed, 36 insertions(+), 35 deletions(-)
--- a/libfreerdp/codec/nsc.c
+++ b/libfreerdp/codec/nsc.c
@@ -30,6 +30,9 @@
#include <winpr/crt.h>
+#include <assert.h>
+#include <winpr/stream.h>
+
#include <freerdp/codec/nsc.h>
#include <freerdp/codec/color.h>
@@ -38,6 +41,14 @@
#include "nsc_sse2.h"
+#define WINPR_ASSERT assert
+
+#if !defined(Stream_CheckAndLogRequiredLengthWLog)
+#define Stream_CheckAndLogRequiredLengthWLog(log, s, len) \
+ Stream_CheckAndLogRequiredLengthWLogEx(log, WLOG_WARN, s, len, "%s(%s:%d)", __FUNCTION__, \
+ __FILE__, __LINE__)
+#endif
+
#ifndef NSC_INIT_SIMD
#define NSC_INIT_SIMD(_nsc_context) \
do \
@@ -175,20 +186,16 @@
static BOOL nsc_rle_decompress_data(NSC_CONTEXT* context)
{
- UINT16 i;
- BYTE* rle;
- UINT32 planeSize;
- UINT32 originalSize;
-
if (!context)
return FALSE;
- rle = context->Planes;
+ BYTE* rle = context->Planes;
+ WINPR_ASSERT(rle);
- for (i = 0; i < 4; i++)
+ for (size_t i = 0; i < 4; i++)
{
- originalSize = context->OrgByteCount[i];
- planeSize = context->PlaneByteCount[i];
+ const UINT32 originalSize = context->OrgByteCount[i];
+ const UINT32 planeSize = context->PlaneByteCount[i];
if (planeSize == 0)
{
@@ -219,64 +226,63 @@
static BOOL nsc_stream_initialize(NSC_CONTEXT* context, wStream* s)
{
- int i;
-
- if (Stream_GetRemainingLength(s) < 20)
+ WINPR_ASSERT(context);
+ WINPR_ASSERT(context->priv);
+ if (!Stream_CheckAndLogRequiredLengthWLog(context->priv->log, s, 20))
return FALSE;
- for (i = 0; i < 4; i++)
+ size_t total = 0;
+ for (size_t i = 0; i < 4; i++)
+ {
Stream_Read_UINT32(s, context->PlaneByteCount[i]);
+ total += context->PlaneByteCount[i];
+ }
Stream_Read_UINT8(s, context->ColorLossLevel); /* ColorLossLevel (1 byte) */
Stream_Read_UINT8(s, context->ChromaSubsamplingLevel); /* ChromaSubsamplingLevel (1 byte) */
Stream_Seek(s, 2); /* Reserved (2 bytes) */
context->Planes = Stream_Pointer(s);
- return TRUE;
+ return Stream_CheckAndLogRequiredLengthWLog(context->priv->log, s, total);
}
static BOOL nsc_context_initialize(NSC_CONTEXT* context, wStream* s)
{
- int i;
- UINT32 length;
- UINT32 tempWidth;
- UINT32 tempHeight;
-
if (!nsc_stream_initialize(context, s))
return FALSE;
- length = context->width * context->height * 4;
+ const size_t blength = context->width * context->height * 4;
if (!context->BitmapData)
{
- context->BitmapData = calloc(1, length + 16);
+ context->BitmapData = calloc(1, blength + 16);
if (!context->BitmapData)
return FALSE;
- context->BitmapDataLength = length;
+ context->BitmapDataLength = blength;
}
- else if (length > context->BitmapDataLength)
+ else if (blength > context->BitmapDataLength)
{
void* tmp;
- tmp = realloc(context->BitmapData, length + 16);
+ tmp = realloc(context->BitmapData, blength + 16);
if (!tmp)
return FALSE;
context->BitmapData = tmp;
- context->BitmapDataLength = length;
+ context->BitmapDataLength = blength;
}
- tempWidth = ROUND_UP_TO(context->width, 8);
- tempHeight = ROUND_UP_TO(context->height, 2);
+ const UINT32 tempWidth = ROUND_UP_TO(context->width, 8);
+ const UINT32 tempHeight = ROUND_UP_TO(context->height, 2);
/* The maximum length a decoded plane can reach in all cases */
- length = tempWidth * tempHeight;
+ const size_t plength = tempWidth * tempHeight;
- if (length > context->priv->PlaneBuffersLength)
+ if (plength > context->priv->PlaneBuffersLength)
{
- for (i = 0; i < 4; i++)
+ for (size_t i = 0; i < 4; i++)
{
- void* tmp = (BYTE*)realloc(context->priv->PlaneBuffers[i], length);
+ void* tmp = (BYTE*)realloc(context->priv->PlaneBuffers[i], plength);
if (!tmp)
return FALSE;
@@ -284,13 +290,11 @@
context->priv->PlaneBuffers[i] = tmp;
}
- context->priv->PlaneBuffersLength = length;
+ context->priv->PlaneBuffersLength = plength;
}
- for (i = 0; i < 4; i++)
- {
+ for (size_t i = 0; i < 4; i++)
context->OrgByteCount[i] = context->width * context->height;
- }
if (context->ChromaSubsamplingLevel)
{
--- a/winpr/include/winpr/stream.h
+++ b/winpr/include/winpr/stream.h
@@ -27,6 +27,7 @@
#include <winpr/wtypes.h>
#include <winpr/endian.h>
#include <winpr/synch.h>
+#include <winpr/wlog.h>
#ifdef __cplusplus
extern "C"
@@ -56,6 +57,14 @@
WINPR_API void Stream_StaticInit(wStream* s, BYTE* buffer, size_t size);
WINPR_API void Stream_Free(wStream* s, BOOL bFreeBuffer);
+
+ WINPR_API BOOL Stream_CheckAndLogRequiredLengthWLogEx(wLog* log, DWORD level, wStream* s,
+ size_t nmemb, size_t size,
+ const char* fmt, ...);
+ WINPR_API BOOL Stream_CheckAndLogRequiredLengthWLogExVa(wLog* log, DWORD level, wStream* s,
+ size_t nmemb, size_t size,
+ const char* fmt, va_list args);
+
static INLINE void Stream_Seek(wStream* s, size_t _offset)
{
s->pointer += (_offset);
--- a/winpr/libwinpr/utils/stream.c
+++ b/winpr/libwinpr/utils/stream.c
@@ -26,6 +26,15 @@
#include <winpr/crt.h>
#include <winpr/stream.h>
+#include <winpr/wlog.h>
+
+#if defined(__GNUC__) || defined(__clang__)
+#define WINPR_ATTR_FORMAT_ARG(pos, args) __attribute__((__format__(__printf__, pos, args)))
+#else
+#define WINPR_ATTR_FORMAT_ARG(pos, args)
+#endif
+
+
BOOL Stream_EnsureCapacity(wStream* s, size_t size)
{
if (s->capacity < size)
@@ -132,3 +141,46 @@
free(s);
}
}
+
+BOOL Stream_CheckAndLogRequiredLengthWLogEx(wLog* log, DWORD level, wStream* s, size_t nmemb,
+ size_t size, const char* fmt, ...)
+{
+ assert(size > 0);
+ const size_t actual = Stream_GetRemainingLength(s) / size;
+
+ if (actual < nmemb)
+ {
+ va_list args;
+
+ va_start(args, fmt);
+ Stream_CheckAndLogRequiredLengthWLogExVa(log, level, s, nmemb, size, fmt, args);
+ va_end(args);
+
+ return FALSE;
+ }
+ return TRUE;
+}
+
+
+WINPR_ATTR_FORMAT_ARG(6, 0)
+BOOL Stream_CheckAndLogRequiredLengthWLogExVa(wLog* log, DWORD level, wStream* s, size_t nmemb,
+ size_t size, const char* fmt, va_list args)
+{
+ assert(size > 0);
+ const size_t actual = Stream_GetRemainingLength(s) / size;
+
+ if (actual < nmemb)
+ {
+ char prefix[1024] = { 0 };
+
+ vsnprintf(prefix, sizeof(prefix), fmt, args);
+
+ WLog_Print(log, level,
+ "[%s] invalid length, got %" PRIuz ", require at least %" PRIuz
+ " [element size=%" PRIuz "]",
+ prefix, actual, nmemb, size);
+ winpr_log_backtrace_ex(log, level, 20);
+ return FALSE;
+ }
+ return TRUE;
+}

View File

@ -1,10 +1,22 @@
Description: Upstream fix for CVE-2023-39354 - Out-Of-Bounds Read in nsc_rle_decompress_data
Origin: https://github.com/FreeRDP/FreeRDP/commit/cd1da25a87358eb3b5512fd259310e95b19a05ec
Description: Upstream fix for CVE-2023-39354 - Out-Of-Bounds Read in nsc_rle_decompress_data
commit 2 of 2.
Origin: https://github.com/FreeRDP/FreeRDP/commit/9a1ee1bae5a9561f5031a7b69129f10458b62d4a
Bug: https://github.com/FreeRDP/FreeRDP/security/advisories/GHSA-c3r2-pxxp-f8r6
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1051638
From 9a1ee1bae5a9561f5031a7b69129f10458b62d4a Mon Sep 17 00:00:00 2001
From: akallabeth <akallabeth@posteo.net>
Date: Tue, 22 Aug 2023 10:48:57 +0200
Subject: [PATCH] [codec,nsc] fix input length validation
(cherry picked from commit e6bb37bea1a645610cc7e030e11fa3ec9e758dc9)
---
libfreerdp/codec/nsc.c | 32 ++++++++++++++++++++++++++++----
libfreerdp/codec/nsc_types.h | 1 +
2 files changed, 29 insertions(+), 4 deletions(-)
--- a/libfreerdp/codec/nsc.c
+++ b/libfreerdp/codec/nsc.c
@@ -111,12 +111,17 @@
@@ -122,12 +122,17 @@
return TRUE;
}
@ -23,7 +35,7 @@ Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1051638
const BYTE value = *in++;
UINT32 len = 0;
@@ -129,17 +134,26 @@
@@ -140,17 +145,26 @@
*out++ = value;
left--;
}
@ -51,7 +63,7 @@ Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1051638
in++;
len = ((UINT32)(*in++));
len |= ((UINT32)(*in++)) << 8U;
@@ -169,6 +183,8 @@
@@ -180,6 +194,8 @@
if ((outSize < 4) || (left < 4))
return FALSE;
@ -60,25 +72,19 @@ Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1051638
memcpy(out, in, 4);
return TRUE;
}
@@ -176,7 +192,7 @@
static BOOL nsc_rle_decompress_data(NSC_CONTEXT* context)
{
UINT16 i;
- BYTE* rle;
+ const BYTE* rle;
UINT32 planeSize;
UINT32 originalSize;
@@ -184,12 +200,16 @@
@@ -189,7 +205,8 @@
if (!context)
return FALSE;
rle = context->Planes;
- BYTE* rle = context->Planes;
+ const BYTE* rle = context->Planes;
+ size_t rleSize = context->PlanesSize;
WINPR_ASSERT(rle);
for (i = 0; i < 4; i++)
{
originalSize = context->OrgByteCount[i];
planeSize = context->PlaneByteCount[i];
for (size_t i = 0; i < 4; i++)
@@ -197,6 +214,9 @@
const UINT32 originalSize = context->OrgByteCount[i];
const UINT32 planeSize = context->PlaneByteCount[i];
+ if (rleSize < planeSize)
+ return FALSE;
@ -86,7 +92,7 @@ Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1051638
if (planeSize == 0)
{
if (context->priv->PlaneBuffersLength < originalSize)
@@ -199,7 +219,7 @@
@@ -206,7 +226,7 @@
}
else if (planeSize < originalSize)
{
@ -95,7 +101,7 @@ Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1051638
context->priv->PlaneBuffersLength, originalSize))
return FALSE;
}
@@ -208,6 +228,9 @@
@@ -215,6 +235,9 @@
if (context->priv->PlaneBuffersLength < originalSize)
return FALSE;
@ -105,6 +111,14 @@ Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1051638
CopyMemory(context->priv->PlaneBuffers[i], rle, originalSize);
}
@@ -242,6 +265,7 @@
Stream_Read_UINT8(s, context->ChromaSubsamplingLevel); /* ChromaSubsamplingLevel (1 byte) */
Stream_Seek(s, 2); /* Reserved (2 bytes) */
context->Planes = Stream_Pointer(s);
+ context->PlanesSize = total;
return Stream_CheckAndLogRequiredLengthWLog(context->priv->log, s, total);
}
--- a/libfreerdp/codec/nsc_types.h
+++ b/libfreerdp/codec/nsc_types.h
@@ -61,6 +61,7 @@

View File

@ -28,3 +28,5 @@
0038-CVE-2023-39352.patch
0039-CVE-2023-39353-part1.patch
0039-CVE-2023-39353-part2.patch
0040-CVE-2023-39354-part1.patch
0040-CVE-2023-39354-part2.patch