helper: Make unhexify() robust on invalid data

The current implementation is not suitable for user provided data
because it does not detect invalid inputs in many cases. For example,
the string "aa0xbb" is successfully converted to the 3 bytes: 0xaa,
0x00 and 0xbb. An other example is "aabi" which is successfully
converted to the 2 bytes: 0xaa and 0x0b. Both are obviously incorrect.

Make unhexify() robust on invalid data and use more appropriate data
types for its parameters. Also, add a small documentation for the
function.

Change-Id: Idb799beb86fc608b066c8a76365021ed44c7f890
Signed-off-by: Marc Schink <openocd-dev@marcschink.de>
Reviewed-on: http://openocd.zylin.com/3792
Tested-by: jenkins
Reviewed-by: Tomas Vanek <vanekt@fbl.cz>

diff --git a/src/helper/binarybuffer.c b/src/helper/binarybuffer.c
index c1e6322658ab1b22b4609e725975ba4fc72b4bbd..26aa8ccede30ce7f236da74ecbaf9aadae6095d8 100644 (file)
--- a/src/helper/binarybuffer.c
+++ b/src/helper/binarybuffer.c
@@ -369,17 +369,42 @@ void bit_copy_discard(struct bit_copy_queue *q)
        }
 }
 
        }
 }
 

-int unhexify(char *bin, const char *hex, int count)
+/**
+ * Convert a string of hexadecimal pairs into its binary
+ * representation.
+ *
+ * @param[out] bin Buffer to store binary representation. The buffer size must
+ *                 be at least @p count.
+ * @param[in] hex String with hexadecimal pairs to convert into its binary
+ *                representation.
+ * @param[in] count Number of hexadecimal pairs to convert.
+ *
+ * @return The number of converted hexadecimal pairs.
+ */
+size_t unhexify(uint8_t *bin, const char *hex, size_t count)

 {
 {

-       int i, tmp;
+       size_t i;
+       char tmp;

 
 

-       for (i = 0; i < count; i++) {
-               if (sscanf(hex + (2 * i), "%02x", &tmp) != 1)
-                       return i;
-               bin[i] = tmp;
+       if (!bin || !hex)
+               return 0;
+
+       memset(bin, 0, count);
+
+       for (i = 0; i < 2 * count; i++) {
+               if (hex[i] >= 'a' && hex[i] <= 'f')
+                       tmp = hex[i] - 'a' + 10;
+               else if (hex[i] >= 'A' && hex[i] <= 'F')
+                       tmp = hex[i] - 'A' + 10;
+               else if (hex[i] >= '0' && hex[i] <= '9')
+                       tmp = hex[i] - '0';
+               else
+                       return i / 2;
+
+               bin[i / 2] |= tmp << (4 * ((i + 1) % 2));

        }
 
        }
 

-       return i;
+       return i / 2;

 }
 
 int hexify(char *hex, const char *bin, int count, int out_maxlen)
 }
 
 int hexify(char *hex, const char *bin, int count, int out_maxlen)

diff --git a/src/helper/binarybuffer.h b/src/helper/binarybuffer.h
index dd0d275abd1ad5e210a8b5871f14191836df1f32..b035779af571b70782956b6873af13e613d6a7d7 100644 (file)
--- a/src/helper/binarybuffer.h
+++ b/src/helper/binarybuffer.h
@@ -234,7 +234,7 @@ void bit_copy_discard(struct bit_copy_queue *q);
 
 /* functions to convert to/from hex encoded buffer
  * used in ti-icdi driver and gdb server */
 
 /* functions to convert to/from hex encoded buffer
  * used in ti-icdi driver and gdb server */

-int unhexify(char *bin, const char *hex, int count);
+size_t unhexify(uint8_t *bin, const char *hex, size_t count);

 int hexify(char *hex, const char *bin, int count, int out_maxlen);
 void buffer_shr(void *_buf, unsigned buf_len, unsigned count);
 
 int hexify(char *hex, const char *bin, int count, int out_maxlen);
 void buffer_shr(void *_buf, unsigned buf_len, unsigned count);
 

diff --git a/src/jtag/drivers/ti_icdi_usb.c b/src/jtag/drivers/ti_icdi_usb.c
index 3db03b032d9579a3e3ffdefc0f5d2f4c85fa77aa..7a6272fdb2da3dff51cfe31e7ae78b8dd1fe982c 100644 (file)
--- a/src/jtag/drivers/ti_icdi_usb.c
+++ b/src/jtag/drivers/ti_icdi_usb.c
@@ -266,7 +266,7 @@ static int icdi_get_cmd_result(void *handle)
 
        if (h->read_buffer[offset] == 'E') {
                /* get error code */
 
        if (h->read_buffer[offset] == 'E') {
                /* get error code */

-               char result;
+               uint8_t result;

                if (unhexify(&result, h->read_buffer + offset + 1, 1) != 1)
                        return ERROR_FAIL;
                return result;
                if (unhexify(&result, h->read_buffer + offset + 1, 1) != 1)
                        return ERROR_FAIL;
                return result;


@@ -328,7 +328,7 @@ static int icdi_usb_version(void *handle)
        }
 
        /* convert reply */
        }
 
        /* convert reply */

-       if (unhexify(version, h->read_buffer + 2, 4) != 4) {
+       if (unhexify((uint8_t *)version, h->read_buffer + 2, 4) != 4) {

                LOG_WARNING("unable to get ICDI version");
                return ERROR_OK;
        }
                LOG_WARNING("unable to get ICDI version");
                return ERROR_OK;
        }


@@ -495,7 +495,7 @@ static int icdi_usb_read_reg(void *handle, int num, uint32_t *val)
 
        /* convert result */
        uint8_t buf[4];
 
        /* convert result */
        uint8_t buf[4];

-       if (unhexify((char *)buf, h->read_buffer + 2, 4) != 4) {
+       if (unhexify(buf, h->read_buffer + 2, 4) != 4) {

                LOG_ERROR("failed to convert result");
                return ERROR_FAIL;
        }
                LOG_ERROR("failed to convert result");
                return ERROR_FAIL;
        }

diff --git a/src/rtos/rtos.c b/src/rtos/rtos.c
index 4c99ad2333b17c53e4f0f772be0cb7fef2a4461e..448c49c060cc96f1690c09ce4cc5dbda03d9e28a 100644 (file)
--- a/src/rtos/rtos.c
+++ b/src/rtos/rtos.c
@@ -213,7 +213,7 @@ int rtos_qsymbol(struct connection *connection, char const *packet, int packet_s
                goto done;
 
        /* Decode any symbol name in the packet*/
                goto done;
 
        /* Decode any symbol name in the packet*/

-       int len = unhexify(cur_sym, strchr(packet + 8, ':') + 1, strlen(strchr(packet + 8, ':') + 1));
+       size_t len = unhexify((uint8_t *)cur_sym, strchr(packet + 8, ':') + 1, strlen(strchr(packet + 8, ':') + 1));

        cur_sym[len] = 0;
 
        if ((strcmp(packet, "qSymbol::") != 0) &&               /* GDB is not offering symbol lookup for the first time */
        cur_sym[len] = 0;
 
        if ((strcmp(packet, "qSymbol::") != 0) &&               /* GDB is not offering symbol lookup for the first time */

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index d09a8be6e454e048391431c24d156dd5aae17b5a..f85fa1bde9d8ffbe82b0190a13cfb2ba5e841328 100644 (file)
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -1449,7 +1449,7 @@ static int gdb_write_memory_packet(struct connection *connection,
 
        LOG_DEBUG("addr: 0x%8.8" PRIx32 ", len: 0x%8.8" PRIx32 "", addr, len);
 
 
        LOG_DEBUG("addr: 0x%8.8" PRIx32 ", len: 0x%8.8" PRIx32 "", addr, len);
 

-       if (unhexify((char *)buffer, separator, len) != (int)len)
+       if (unhexify(buffer, separator, len) != len)

                LOG_ERROR("unable to decode memory packet");
 
        retval = target_write_buffer(target, addr, len, buffer);
                LOG_ERROR("unable to decode memory packet");
 
        retval = target_write_buffer(target, addr, len, buffer);


@@ -2277,7 +2277,7 @@ static int gdb_query_packet(struct connection *connection,
                if (packet_size > 6) {
                        char *cmd;
                        cmd = malloc((packet_size - 6) / 2 + 1);
                if (packet_size > 6) {
                        char *cmd;
                        cmd = malloc((packet_size - 6) / 2 + 1);

-                       int len = unhexify(cmd, packet + 6, (packet_size - 6) / 2);
+                       size_t len = unhexify((uint8_t *)cmd, packet + 6, (packet_size - 6) / 2);

                        cmd[len] = 0;
 
                        /* We want to print all debug output to GDB connection */
                        cmd[len] = 0;
 
                        /* We want to print all debug output to GDB connection */