Skip to content

Commit

Permalink
device.c : remove strcpy & sprintf, and move to safe functions
Browse files Browse the repository at this point in the history
We used strcpy & sprintf in a few places, which opens up classic
buffer overflow issues, as described in:
https://cwe.mitre.org/data/definitions/120.html

This adds the recent length checking, to make sure as the buffer
descreases in size, it is managed properly. If we ever think we
run out of space, we will no longer buffer overflow.

Tested on Pluto and M2k to see if this introduces issues, and couldn't
find anything.

Signed-off-by: Robin Getz <[email protected]>
  • Loading branch information
rgetz committed Apr 22, 2020
1 parent 1e91e5d commit 2f55cd4
Showing 1 changed file with 37 additions and 22 deletions.
59 changes: 37 additions & 22 deletions device.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,63 +150,78 @@ char * iio_device_get_xml(const struct iio_device *dev, size_t *length)
if (!str)
goto err_free_debug_attrs;
eptr = str + len;
ptr = str;

iio_snprintf(str, len, "<device id=\"%s\"", dev->id);
ptr = strrchr(str, '\0');
len = eptr - ptr;
if (len > 0) {
iio_snprintf(str, len, "<device id=\"%s\"", dev->id);
ptr = strrchr(str, '\0');
len = eptr - ptr;
}

if (dev->name) {
sprintf(ptr, " name=\"%s\"", dev->name);
if (dev->name && len > 0) {
iio_snprintf(ptr, len, " name=\"%s\"", dev->name);
ptr = strrchr(ptr, '\0');
len = eptr - ptr;
}

strcpy(ptr, " >");
ptr += 2;
len -= 2;
if (len > 0) {
iio_strlcpy(ptr, " >", len);
ptr += 2;
len -= 2;
}

for (i = 0; i < dev->nb_channels; i++) {
strcpy(ptr, channels[i]);
ptr += channels_len[i];
len -= channels_len[i];
if (len > 0) {
iio_strlcpy(ptr, channels[i], len);
ptr += channels_len[i];
len -= channels_len[i];
}
free(channels[i]);
}

free(channels);
free(channels_len);

for (i = 0; i < dev->nb_attrs; i++) {
strcpy(ptr, attrs[i]);
ptr += attrs_len[i];
len -= attrs_len[i];
if (len > 0) {
iio_strlcpy(ptr, attrs[i], len);
ptr += attrs_len[i];
len -= attrs_len[i];
}
free(attrs[i]);
}

free(attrs);
free(attrs_len);

for (i = 0; i < dev->nb_buffer_attrs; i++) {
strcpy(ptr, buffer_attrs[i]);
ptr += buffer_attrs_len[i];
len -= buffer_attrs_len[i];
if (len > 0) {
iio_strlcpy(ptr, buffer_attrs[i], len);
ptr += buffer_attrs_len[i];
len -= buffer_attrs_len[i];
}
free(buffer_attrs[i]);
}

free(buffer_attrs);
free(buffer_attrs_len);

for (i = 0; i < dev->nb_debug_attrs; i++) {
strcpy(ptr, debug_attrs[i]);
ptr += debug_attrs_len[i];
len -= debug_attrs_len[i];
if (len > 0) {
iio_strlcpy(ptr, debug_attrs[i], len);
ptr += debug_attrs_len[i];
len -= debug_attrs_len[i];
}
free(debug_attrs[i]);
}

free(debug_attrs);
free(debug_attrs_len);

strcpy(ptr, "</device>");
len -= sizeof("</device>") - 1;
if (len > 0) {
iio_strlcpy(ptr, "</device>", len);
len -= sizeof("</device>") - 1;
}

*length = ptr - str + sizeof("</device>") - 1;

Expand Down

0 comments on commit 2f55cd4

Please sign in to comment.