Skip to content

Commit 2f55cd4

Browse files
committed
device.c : remove strcpy & sprintf, and move to safe functions
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]>
1 parent 1e91e5d commit 2f55cd4

File tree

1 file changed

+37
-22
lines changed

1 file changed

+37
-22
lines changed

device.c

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -150,63 +150,78 @@ char * iio_device_get_xml(const struct iio_device *dev, size_t *length)
150150
if (!str)
151151
goto err_free_debug_attrs;
152152
eptr = str + len;
153+
ptr = str;
153154

154-
iio_snprintf(str, len, "<device id=\"%s\"", dev->id);
155-
ptr = strrchr(str, '\0');
156-
len = eptr - ptr;
155+
if (len > 0) {
156+
iio_snprintf(str, len, "<device id=\"%s\"", dev->id);
157+
ptr = strrchr(str, '\0');
158+
len = eptr - ptr;
159+
}
157160

158-
if (dev->name) {
159-
sprintf(ptr, " name=\"%s\"", dev->name);
161+
if (dev->name && len > 0) {
162+
iio_snprintf(ptr, len, " name=\"%s\"", dev->name);
160163
ptr = strrchr(ptr, '\0');
161164
len = eptr - ptr;
162165
}
163166

164-
strcpy(ptr, " >");
165-
ptr += 2;
166-
len -= 2;
167+
if (len > 0) {
168+
iio_strlcpy(ptr, " >", len);
169+
ptr += 2;
170+
len -= 2;
171+
}
167172

168173
for (i = 0; i < dev->nb_channels; i++) {
169-
strcpy(ptr, channels[i]);
170-
ptr += channels_len[i];
171-
len -= channels_len[i];
174+
if (len > 0) {
175+
iio_strlcpy(ptr, channels[i], len);
176+
ptr += channels_len[i];
177+
len -= channels_len[i];
178+
}
172179
free(channels[i]);
173180
}
174181

175182
free(channels);
176183
free(channels_len);
177184

178185
for (i = 0; i < dev->nb_attrs; i++) {
179-
strcpy(ptr, attrs[i]);
180-
ptr += attrs_len[i];
181-
len -= attrs_len[i];
186+
if (len > 0) {
187+
iio_strlcpy(ptr, attrs[i], len);
188+
ptr += attrs_len[i];
189+
len -= attrs_len[i];
190+
}
182191
free(attrs[i]);
183192
}
184193

185194
free(attrs);
186195
free(attrs_len);
187196

188197
for (i = 0; i < dev->nb_buffer_attrs; i++) {
189-
strcpy(ptr, buffer_attrs[i]);
190-
ptr += buffer_attrs_len[i];
191-
len -= buffer_attrs_len[i];
198+
if (len > 0) {
199+
iio_strlcpy(ptr, buffer_attrs[i], len);
200+
ptr += buffer_attrs_len[i];
201+
len -= buffer_attrs_len[i];
202+
}
192203
free(buffer_attrs[i]);
193204
}
194205

195206
free(buffer_attrs);
196207
free(buffer_attrs_len);
197208

198209
for (i = 0; i < dev->nb_debug_attrs; i++) {
199-
strcpy(ptr, debug_attrs[i]);
200-
ptr += debug_attrs_len[i];
201-
len -= debug_attrs_len[i];
210+
if (len > 0) {
211+
iio_strlcpy(ptr, debug_attrs[i], len);
212+
ptr += debug_attrs_len[i];
213+
len -= debug_attrs_len[i];
214+
}
202215
free(debug_attrs[i]);
203216
}
204217

205218
free(debug_attrs);
206219
free(debug_attrs_len);
207220

208-
strcpy(ptr, "</device>");
209-
len -= sizeof("</device>") - 1;
221+
if (len > 0) {
222+
iio_strlcpy(ptr, "</device>", len);
223+
len -= sizeof("</device>") - 1;
224+
}
210225

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

0 commit comments

Comments
 (0)