Skip to content

Commit

Permalink
CVE-2023-42822
Browse files Browse the repository at this point in the history 
- font_items in struct xrdp_font renamed to chars to catch all
  accesses to it. This name is consistent with the type of
  the array elements (struct xrdp_font_char).
- Additional fields added to struct xrdp_font to allow for range
  checking and for a default character to be provided
- Additional checks and logic added to xrdp_font_create()
- New macro XRDP_FONT_GET_CHAR() added to perform checked access
  to chars field in struct xrdp_font
  • Loading branch information
@matt335672
matt335672 committed Sep 26, 2023
1 parent 98ad496 commit bc3ea01
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 28 deletions.
9 changes: 9 additions & 0 deletions xrdp/xrdp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,15 @@ xrdp_font_delete(struct xrdp_font *self);
int
xrdp_font_item_compare(struct xrdp_font_char *font1,
struct xrdp_font_char *font2);
/**
* Gets a checked xrdp_font_char from a font
* @param f Font
* @param c32 Unicode codepoint
*/
#define XRDP_FONT_GET_CHAR(f, c32) \
(((unsigned int)(c32) >= ' ') && ((unsigned int)(c32) < (f)->char_count) \
? ((f)->chars + (unsigned int)(c32)) \
: (f)->default_char)

/* funcs.c */
int
Expand Down
130 changes: 108 additions & 22 deletions xrdp/xrdp_font.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ static char w_char[] =
};
#endif

// Unicode definitions
#define UNICODE_WHITE_SQUARE 0x25a1

// First character allocated in the 'struct xrdp_font.chars' array
#define FIRST_CHAR ' '

/*****************************************************************************/
/**
* Parses the fv1_select configuration value to get the font to use,
Expand Down Expand Up @@ -145,8 +151,8 @@ xrdp_font_create(struct xrdp_wm *wm, unsigned int dpi)
int fd;
int b;
int i;
int index;
int datasize;
unsigned int char_count;
unsigned int datasize; // Size of glyph data on disk
int file_size;
struct xrdp_font_char *f;
const char *file_path;
Expand Down Expand Up @@ -207,17 +213,39 @@ xrdp_font_create(struct xrdp_wm *wm, unsigned int dpi)
}

self = (struct xrdp_font *)g_malloc(sizeof(struct xrdp_font), 1);
if (self == NULL)
{
LOG(LOG_LEVEL_ERROR, "xrdp_font_create: "
"Can't allocate memory for font");
return self;
}
self->wm = wm;
make_stream(s);
init_stream(s, file_size + 1024);
fd = g_file_open_ro(file_path);

if (fd != -1)
if (fd < 0)
{
LOG(LOG_LEVEL_ERROR,
"xrdp_font_create: Can't open %s - %s", file_path,
g_get_strerror());
g_free(self);
self = NULL;
}
else
{
b = g_file_read(fd, s->data, file_size + 1024);
g_file_close(fd);

if (b > 0)
// Got at least a header?
if (b < (4 + 32 + 2 + 2 + 2 + 2 + 4))
{
LOG(LOG_LEVEL_ERROR,
"xrdp_font_create: Font %s is truncated", file_path);
g_free(self);
self = NULL;
}
else
{
s->end = s->data + b;
in_uint8s(s, 4);
Expand All @@ -227,11 +255,27 @@ xrdp_font_create(struct xrdp_wm *wm, unsigned int dpi)
in_uint16_le(s, self->body_height);
in_sint16_le(s, min_descender);
in_uint8s(s, 4);
index = 32;
char_count = FIRST_CHAR;

while (s_check_rem(s, 16))
while (!s_check_end(s))
{
f = self->font_items + index;
if (!s_check_rem(s, 16))
{
LOG(LOG_LEVEL_WARNING,
"xrdp_font_create: "
"Can't parse header for character U+%X", char_count);
break;
}

if (char_count >= MAX_FONT_CHARS)
{
LOG(LOG_LEVEL_WARNING,
"xrdp_font_create: "
"Ignoring characters >= U+%x", MAX_FONT_CHARS);
break;
}

f = self->chars + char_count;
in_sint16_le(s, i);
f->width = i;
in_sint16_le(s, i);
Expand All @@ -249,9 +293,19 @@ xrdp_font_create(struct xrdp_wm *wm, unsigned int dpi)
if (datasize < 0 || datasize > 512)
{
/* shouldn't happen */
LOG(LOG_LEVEL_ERROR, "error in xrdp_font_create, datasize wrong "
"width %d, height %d, datasize %d, index %d",
f->width, f->height, datasize, index);
LOG(LOG_LEVEL_ERROR,
"xrdp_font_create: "
"datasize for U+%x wrong "
"width %d, height %d, datasize %d",
char_count, f->width, f->height, datasize);
break;
}

if (!s_check_rem(s, datasize))
{
LOG(LOG_LEVEL_ERROR,
"xrdp_font_create: "
"Not enough data for character U+%X", char_count);
break;
}

Expand All @@ -261,25 +315,57 @@ xrdp_font_create(struct xrdp_wm *wm, unsigned int dpi)
* that it can be added to the glyph cache if required */
f->width = 1;
f->height = 1;

/* GOTCHA - we need to allocate more than one byte in
* memory for this glyph */
f->data = (char *)g_malloc(FONT_DATASIZE(f), 1);
}
else if (s_check_rem(s, datasize))
else
{
f->data = (char *)g_malloc(datasize, 0);
in_uint8a(s, f->data, datasize);
}
else

if (f->data == NULL)
{
LOG(LOG_LEVEL_ERROR, "error in xrdp_font_create");
LOG(LOG_LEVEL_ERROR,
"xrdp_font_create: "
"Allocation error for character U+%X", char_count);
break;
}
index++;
in_uint8a(s, f->data, datasize);

++char_count;
}

if (self->body_height == 0 && index > 32)
self->char_count = char_count;
if (char_count <= FIRST_CHAR)
{
/* We read no characters from the font */
xrdp_font_delete(self);
self = NULL;
}
else
{
/* Older font made for xrdp v0.9.x. Synthesise this
* value from the first glyph */
self->body_height = -self->font_items[32].baseline + 1;
if (self->body_height == 0)
{
/* Older font made for xrdp v0.9.x. Synthesise this
* value from the first glyph */
self->body_height = -self->chars[FIRST_CHAR].baseline + 1;
}

// Find a default glyph
if (char_count > UNICODE_WHITE_SQUARE)
{
self->default_char = &self->chars[UNICODE_WHITE_SQUARE];
}
else if (char_count > '?')
{
self->default_char = &self->chars['?'];
}
else
{
self->default_char = &self->chars[FIRST_CHAR];
}
}
}
}
Expand All @@ -302,16 +388,16 @@ xrdp_font_create(struct xrdp_wm *wm, unsigned int dpi)
void
xrdp_font_delete(struct xrdp_font *self)
{
int i;
unsigned int i;

if (self == 0)
{
return;
}

for (i = 0; i < NUM_FONTS; i++)
for (i = FIRST_CHAR; i < self->char_count; i++)
{
g_free(self->font_items[i].data);
g_free(self->chars[i].data);
}

g_free(self);
Expand Down
8 changes: 4 additions & 4 deletions xrdp/xrdp_painter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ xrdp_painter_text_width(struct xrdp_painter *self, const char *text)

for (index = 0; index < len; index++)
{
font_item = self->font->font_items + wstr[index];
font_item = XRDP_FONT_GET_CHAR(self->font, wstr[index]);
rv = rv + font_item->incby;
}

Expand Down Expand Up @@ -837,7 +837,7 @@ xrdp_painter_draw_text(struct xrdp_painter *self,
total_height = 0;
for (index = 0; index < len; index++)
{
font_item = font->font_items + wstr[index];
font_item = XRDP_FONT_GET_CHAR(font, wstr[index]);
k = font_item->incby;
total_width += k;
/* Use the nominal height of the font to work out the
Expand Down Expand Up @@ -875,7 +875,7 @@ xrdp_painter_draw_text(struct xrdp_painter *self,
draw_rect.bottom - draw_rect.top);
for (index = 0; index < len; index++)
{
font_item = font->font_items + wstr[index];
font_item = XRDP_FONT_GET_CHAR(font, wstr[index]);
g_memset(&pat, 0, sizeof(pat));
pat.format = PT_FORMAT_c1;
pat.width = font_item->width;
Expand Down Expand Up @@ -917,7 +917,7 @@ xrdp_painter_draw_text(struct xrdp_painter *self,

for (index = 0; index < len; index++)
{
font_item = font->font_items + wstr[index];
font_item = XRDP_FONT_GET_CHAR(font, wstr[index]);
i = xrdp_cache_add_char(self->wm->cache, font_item);
f = HIWORD(i);
c = LOWORD(i);
Expand Down
8 changes: 6 additions & 2 deletions xrdp/xrdp_types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ struct xrdp_bitmap
int crc16;
};

#define NUM_FONTS 0x4e00
#define MAX_FONT_CHARS 0x4e00
#define DEFAULT_FONT_NAME "sans-10.fv1"
#define DEFAULT_FONT_PIXEL_SIZE 16
#define DEFAULT_FV1_SELECT "130:sans-18.fv1,0:" DEFAULT_FONT_NAME
Expand All @@ -669,7 +669,11 @@ struct xrdp_bitmap
struct xrdp_font
{
struct xrdp_wm *wm;
struct xrdp_font_char font_items[NUM_FONTS];
// Font characters, accessed by Unicode codepoint. The first 32
// entries are unused.
struct xrdp_font_char chars[MAX_FONT_CHARS];
unsigned int char_count; // # elements in above array
struct xrdp_font_char *default_char; // Pointer into above array
char name[32];
int size;
/** Body height in pixels */
Expand Down

0 comments on commit bc3ea01

Please sign in to comment.