From 9772e3b5dc69bfda334a73b1d370d1743e3565bb Mon Sep 17 00:00:00 2001 From: Daniel Willmann Date: Wed, 12 Dec 2012 17:23:09 +0000 Subject: [PATCH] efl: Fix possible memory corruption in ecore xrandr EDID functions Report from Klocwork. I checked that the actual max size of the name is 13 bytes. Now we allocate one more to hold the terminating NULL byte and not write into unallocated memory. Signed-off-by: Daniel Willmann SVN revision: 80773 --- ChangeLog | 1 + NEWS | 1 + src/lib/ecore_x/xcb/ecore_xcb_randr.c | 5 ++--- src/lib/ecore_x/xlib/ecore_x_randr_12_edid.c | 12 ++++++------ 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/ChangeLog b/ChangeLog index 87e85b0974..74a6b658fb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,7 @@ 2012-12-12 Daniel Willmann * Fix possible buffer overflow in functions relying on EET_T_LAST. + * Fix possible memory corruption in xrandr EDID functions. 2012-12-12 Cedric Bail diff --git a/NEWS b/NEWS index 4620111b41..9de63b6e7b 100644 --- a/NEWS +++ b/NEWS @@ -77,3 +77,4 @@ Fixes: * Fix leak in eet_pbkdf2_sha1 with OpenSSL. * Fix the gl line incorrect position drawing. * Fix possible buffer overflow in functions relying on EET_T_LAST + * Fix possible memory corruption in xrandr EDID functions. diff --git a/src/lib/ecore_x/xcb/ecore_xcb_randr.c b/src/lib/ecore_x/xcb/ecore_xcb_randr.c index a2a4e6271f..f3ae9b5f28 100644 --- a/src/lib/ecore_x/xcb/ecore_xcb_randr.c +++ b/src/lib/ecore_x/xcb/ecore_xcb_randr.c @@ -2761,12 +2761,11 @@ ecore_x_randr_edid_display_name_get(unsigned char *edid, unsigned long edid_leng edid_name = (const char *)block + _ECORE_X_RANDR_EDID_OFFSET_DESCRIPTOR_BLOCK_CONTENT; name = - malloc(sizeof(char) * - _ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX); + malloc(_ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX + 1); if (!name) return NULL; strncpy(name, edid_name, - (_ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX - 1)); + _ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX); name[_ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX] = 0; for (p = name; *p; p++) if ((*p < ' ') || (*p > '~')) *p = 0; diff --git a/src/lib/ecore_x/xlib/ecore_x_randr_12_edid.c b/src/lib/ecore_x/xlib/ecore_x_randr_12_edid.c index 5bda332a06..4c37a2c9c3 100644 --- a/src/lib/ecore_x/xlib/ecore_x_randr_12_edid.c +++ b/src/lib/ecore_x/xlib/ecore_x_randr_12_edid.c @@ -184,9 +184,9 @@ ecore_x_randr_edid_display_name_get(unsigned char *edid, const char *edid_name; edid_name = (const char *)block + _ECORE_X_RANDR_EDID_OFFSET_DESCRIPTOR_BLOCK_CONTENT; - name = malloc(sizeof(char) * _ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX); + name = malloc(_ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX + 1); if (!name) return NULL; - strncpy(name, edid_name, (_ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX - 1)); + strncpy(name, edid_name, _ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX); name[_ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX] = 0; for (p = name; *p; p++) { @@ -288,9 +288,9 @@ ecore_x_randr_edid_display_ascii_get(unsigned char *edid, * TODO: Two of these in a row, in the third and fourth slots, * seems to be specified by SPWG: http://www.spwg.org/ */ - ascii = malloc(sizeof(char) * _ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX); + ascii = malloc(_ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX + 1); if (!ascii) return NULL; - strncpy(ascii, edid_ascii, (_ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX - 1)); + strncpy(ascii, edid_ascii, _ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX); ascii[_ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX] = 0; for (p = ascii; *p; p++) { @@ -321,9 +321,9 @@ ecore_x_randr_edid_display_serial_get(unsigned char *edid, * TODO: Two of these in a row, in the third and fourth slots, * seems to be specified by SPWG: http://www.spwg.org/ */ - serial = malloc(sizeof(char) * _ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX); + serial = malloc(_ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX + 1); if (!serial) return NULL; - strncpy(serial, edid_serial, (_ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX - 1)); + strncpy(serial, edid_serial, _ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX); serial[_ECORE_X_RANDR_EDID_DISPLAY_DESCRIPTOR_BLOCK_CONTENT_LENGTH_MAX] = 0; for (p = serial; *p; p++) {