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 <d.willmann@samsung.com>
SVN revision: 80773

diff --git a/ChangeLog b/ChangeLog
index 87e85b0..74a6b65 100644 (file)
--- 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 4620111..9de63b6 100644 (file)
--- 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 a2a4e62..f3ae9b5 100644 (file)
--- 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 5bda332..4c37a2c 100644 (file)
--- 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++)
              {