From db72b7361368aa4d549579d11d7ebd67707c79d5 Mon Sep 17 00:00:00 2001 From: Robin Getz Date: Fri, 24 Apr 2020 15:38:21 -0400 Subject: [PATCH 1/2] iiod-client.c: rework iiod_client_open_unlocked to remove strcpy https://cwe.mitre.org/data/definitions/120.html defines strcpy as potentially dangerous, and is included in Microsoft's banned.h This re-works the function to remove it, and use iio_strlcpy instead. tested remotely and locally. Signed-off-by: Robin Getz --- iiod-client.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/iiod-client.c b/iiod-client.c index d18475438..ebeb64486 100644 --- a/iiod-client.c +++ b/iiod-client.c @@ -202,7 +202,7 @@ int iiod_client_get_version(struct iiod_client *client, void *desc, if (minor) *minor = (unsigned int) min; if (git_tag) - strncpy(git_tag, ptr, 8); + iio_strlcpy(git_tag, ptr, 8); return 0; } @@ -534,17 +534,24 @@ int iiod_client_open_unlocked(struct iiod_client *client, void *desc, { char buf[1024], *ptr; size_t i; + ssize_t len; - iio_snprintf(buf, sizeof(buf), "OPEN %s %lu ", + len = sizeof(buf); + len -= iio_snprintf(buf, len, "OPEN %s %lu ", iio_device_get_id(dev), (unsigned long) samples_count); ptr = buf + strlen(buf); for (i = dev->words; i > 0; i--, ptr += 8) { - iio_snprintf(ptr, (ptr - buf) + i * 8, "%08" PRIx32, + len -= iio_snprintf(ptr, len, "%08" PRIx32, dev->mask[i - 1]); } - strcpy(ptr, cyclic ? " CYCLIC\r\n" : "\r\n"); + len -= iio_strlcpy(ptr, cyclic ? " CYCLIC\r\n" : "\r\n", len); + + if (len < 0) { + IIO_ERROR("strlength problem in iiod_client_open_unlocked\n"); + return -ENOMEM; + } return iiod_client_exec_command(client, desc, buf); } From 3f305ee3f0ada31d4d26ac97f349912869825cfe Mon Sep 17 00:00:00 2001 From: Robin Getz Date: Fri, 24 Apr 2020 16:03:20 -0400 Subject: [PATCH 2/2] replace strcpy and snprintf in remaining user in libiio https://cwe.mitre.org/data/definitions/120.html defines strcpy as potentially dangerous, and is included in Microsoft's banned.h snprintf has a iio_ special version that uses sprintf_s with Microsoft Compilers. sprintf_s guarantees that the buffer will be null-terminated unless the buffer size is zero (among other things). Linux snprintf does this by default. replace strncpy with iio_strlcpy taking advantage of mandatory null termination in iio_strlcpy replace snprintf with iio_snprintf Signed-off-by: Robin Getz --- dns_sd.c | 2 +- dns_sd_bonjour.c | 4 ++-- iiod/ops.c | 2 +- iiod/usbd.c | 6 +++--- local.c | 3 +-- network.c | 2 +- utilities.c | 3 +-- 7 files changed, 10 insertions(+), 12 deletions(-) diff --git a/dns_sd.c b/dns_sd.c index 1602973d7..11167c553 100644 --- a/dns_sd.c +++ b/dns_sd.c @@ -290,7 +290,7 @@ int dnssd_discover_host(char *addr_str, size_t addr_len, uint16_t *port) if (ddata) { *port = ddata->port; - strncpy(addr_str, ddata->addr_str, addr_len); + iio_strlcpy(addr_str, ddata->addr_str, addr_len); } host_fail: diff --git a/dns_sd_bonjour.c b/dns_sd_bonjour.c index 037bf68be..424538226 100644 --- a/dns_sd_bonjour.c +++ b/dns_sd_bonjour.c @@ -144,9 +144,9 @@ static void __cfnet_browser_cb ( dd->port = port; dd->hostname = strdup(hostname); if (have_v4) { - strncpy(dd->addr_str, address_v4, sizeof(dd->addr_str)); + iio_strlcpy(dd->addr_str, address_v4, sizeof(dd->addr_str)); } else if(have_v6) { - strncpy(dd->addr_str, address_v6, sizeof(dd->addr_str)); + iio_strlcpy(dd->addr_str, address_v6, sizeof(dd->addr_str)); } IIO_DEBUG("DNS SD: added %s (%s:%d)\n", hostname, dd->addr_str, port); diff --git a/iiod/ops.c b/iiod/ops.c index 977665731..e2e3add99 100644 --- a/iiod/ops.c +++ b/iiod/ops.c @@ -1262,7 +1262,7 @@ ssize_t get_trigger(struct parser_pdata *pdata, struct iio_device *dev) ret = strlen(trigger->name); print_value(pdata, ret); - snprintf(buf, sizeof(buf), "%s\n", trigger->name); + iio_snprintf(buf, sizeof(buf), "%s\n", trigger->name); ret = write_all(pdata, buf, ret + 1); } else { print_value(pdata, ret); diff --git a/iiod/usbd.c b/iiod/usbd.c index 2a990af6f..6321fe546 100644 --- a/iiod/usbd.c +++ b/iiod/usbd.c @@ -106,14 +106,14 @@ static int usb_open_pipe(struct usbd_pdata *pdata, unsigned int pipe_id) * before opening the endpoints again. */ thread_pool_stop_and_wait(pdata->pool[pipe_id]); - snprintf(buf, sizeof(buf), "%s/ep%u", pdata->ffs, pipe_id * 2 + 1); + iio_snprintf(buf, sizeof(buf), "%s/ep%u", pdata->ffs, pipe_id * 2 + 1); cpdata->ep_out = open(buf, O_WRONLY); if (cpdata->ep_out < 0) { err = -errno; goto err_free_cpdata; } - snprintf(buf, sizeof(buf), "%s/ep%u", pdata->ffs, pipe_id * 2 + 2); + iio_snprintf(buf, sizeof(buf), "%s/ep%u", pdata->ffs, pipe_id * 2 + 2); cpdata->ep_in = open(buf, O_RDONLY); if (cpdata->ep_in < 0) { err = -errno; @@ -361,7 +361,7 @@ int start_usb_daemon(struct iio_context *ctx, const char *ffs, goto err_free_pdata_pool; } - snprintf(buf, sizeof(buf), "%s/ep0", ffs); + iio_snprintf(buf, sizeof(buf), "%s/ep0", ffs); pdata->ep0_fd = open(buf, O_RDWR); if (pdata->ep0_fd < 0) { diff --git a/local.c b/local.c index a57adda41..a11fb94aa 100644 --- a/local.c +++ b/local.c @@ -223,8 +223,7 @@ static int set_channel_name(struct iio_channel *chn) name = malloc(prefix_len); if (!name) return -ENOMEM; - strncpy(name, attr0, prefix_len - 1); - name[prefix_len - 1] = '\0'; + iio_strlcpy(name, attr0, prefix_len - 1); IIO_DEBUG("Setting name of channel %s to %s\n", chn->id, name); chn->name = name; diff --git a/network.c b/network.c index e979f2519..28e08550a 100644 --- a/network.c +++ b/network.c @@ -1424,7 +1424,7 @@ struct iio_context * network_create_context(const char *host) inet_ntop(AF_INET, &in->sin_addr, description, INET_ADDRSTRLEN); #else char *tmp = inet_ntoa(in->sin_addr); - strncpy(description, tmp, len); + iio_strlcpy(description, tmp, len); #endif } diff --git a/utilities.c b/utilities.c index 2178f194c..a41ed7cf7 100644 --- a/utilities.c +++ b/utilities.c @@ -180,8 +180,7 @@ void iio_library_get_version(unsigned int *major, if (minor) *minor = LIBIIO_VERSION_MINOR; if (git_tag) { - strncpy(git_tag, LIBIIO_VERSION_GIT, 8); - git_tag[7] = '\0'; + iio_strlcpy(git_tag, LIBIIO_VERSION_GIT, 8); } }