From db96d34c0ecd9b5061781caf60dcb474a1ba73f2 Mon Sep 17 00:00:00 2001 From: Parichay Kapoor Date: Thu, 26 Sep 2019 12:03:23 +0900 Subject: [PATCH] [svace/iio] security fixes related to sscanf 1. Fix security issue related with sscanf. Use g_ascii_str instead Issue: WID:7938408 Use of vulnerable function 'sscanf' at tensor_src_iio.c:690. This function is unsafe. Issue: WID:7938409 Use of vulnerable function 'sscanf' at tensor_src_iio.c:723. This function is unsafe. 2. Add comments for switch cases where breaks have been skipped Issue: Missing break in the switch case Signed-off-by: Parichay Kapoor --- gst/nnstreamer/tensor_source/tensor_src_iio.c | 82 +++++++++++++++++++-------- tests/nnstreamer_source/unittest_src_iio.cpp | 5 ++ 2 files changed, 64 insertions(+), 23 deletions(-) diff --git a/gst/nnstreamer/tensor_source/tensor_src_iio.c b/gst/nnstreamer/tensor_source/tensor_src_iio.c index 369bbd9..0afa6d4 100644 --- a/gst/nnstreamer/tensor_source/tensor_src_iio.c +++ b/gst/nnstreamer/tensor_source/tensor_src_iio.c @@ -681,13 +681,15 @@ gst_tensor_src_iio_get_float_from_file (const gchar * dirname, { gchar *filename, *filepath, *file_contents = NULL; + errno = 0; filename = g_strdup_printf ("%s%s", name, suffix); filepath = g_build_filename (dirname, filename, NULL); if (!g_file_get_contents (filepath, &file_contents, NULL, NULL)) { GST_INFO ("Unable to retrieve data from file %s.", filename); } else { - if (!sscanf (file_contents, "%f", value)) { + *value = (gfloat) g_ascii_strtod (file_contents, NULL); + if (errno != 0) { GST_ERROR ("Error in parsing float."); goto failure; } @@ -711,48 +713,82 @@ failure: * @param[in] contents Contains type unparsed information to be set * @return True if info was successfully set, false is info is not be parsed * correctly + * @detail The format for the contents is expected to be of format + * [be|le]:[s|u]bits/storagebits[>>shift] */ static gboolean gst_tensor_src_iio_set_channel_type (GstTensorSrcIIOChannelProperties * prop, const gchar * contents) { gchar endianchar = '\0', signchar = '\0'; - gint arguments_filled; - gboolean ret = TRUE; - arguments_filled = - sscanf (contents, "%ce:%c%u/%u>>%u", &endianchar, &signchar, - &prop->used_bits, &prop->storage_bits, &prop->shift); - if (prop->storage_bits > 0) { - prop->storage_bytes = ((prop->storage_bits - 1) >> 3) + 1; - } else { - GST_WARNING ("Storage bits are 0 for channel %s.", prop->name); - prop->storage_bytes = 0; - } - - prop->mask = G_MAXUINT64 >> (64 - prop->used_bits); - - /** checks to verify device channel type settings */ - g_return_val_if_fail (arguments_filled == 5, FALSE); - g_return_val_if_fail (prop->storage_bytes <= 8, FALSE); - g_return_val_if_fail (prop->storage_bits >= prop->used_bits, FALSE); - g_return_val_if_fail (prop->storage_bits > prop->shift, FALSE); + gchar *start, *end; + guint base = 10; + errno = 0; + /** check endian */ + endianchar = contents[0]; if (endianchar == 'b') { prop->big_endian = TRUE; } else if (endianchar == 'l') { prop->big_endian = FALSE; } else { - ret = FALSE; + goto exit_fail; } + + /** verify static parts of the contents */ + g_return_val_if_fail (contents[1] == 'e', FALSE); + g_return_val_if_fail (contents[2] == ':', FALSE); + + /** check sign */ + signchar = contents[3]; if (signchar == 's') { prop->is_signed = TRUE; } else if (signchar == 'u') { prop->is_signed = FALSE; } else { - ret = FALSE; + goto exit_fail; } - return ret; + /** used bits */ + start = (gchar *) contents + 4; + prop->used_bits = (guint) g_ascii_strtoull (start, &end, base); + if (errno != 0) { + goto exit_fail; + } + /** verify static parts of the contents */ + g_return_val_if_fail (end[0] == '/', FALSE); + prop->mask = G_MAXUINT64 >> (64 - prop->used_bits); + + /** storage bits */ + start = &end[1]; + prop->storage_bits = (guint) g_ascii_strtoull (start, &end, base); + if (errno != 0) { + goto exit_fail; + } + /** verify static parts of the contents */ + g_return_val_if_fail (end[0] == '>', FALSE); + g_return_val_if_fail (end[1] == '>', FALSE); + g_return_val_if_fail (prop->storage_bits >= prop->used_bits, FALSE); + + if (prop->storage_bits > 0) { + prop->storage_bytes = ((prop->storage_bits - 1) >> 3) + 1; + g_return_val_if_fail (prop->storage_bytes <= 8, FALSE); + } else { + GST_WARNING ("Storage bits are 0 for channel %s.", prop->name); + prop->storage_bytes = 0; + } + + start = &end[2]; + prop->shift = (guint) g_ascii_strtoull (start, &end, base); + if (errno != 0) { + goto exit_fail; + } + g_return_val_if_fail (prop->storage_bits > prop->shift, FALSE); + + return TRUE; + +exit_fail: + return FALSE; } /** diff --git a/tests/nnstreamer_source/unittest_src_iio.cpp b/tests/nnstreamer_source/unittest_src_iio.cpp index 383a551..e35b5c3 100644 --- a/tests/nnstreamer_source/unittest_src_iio.cpp +++ b/tests/nnstreamer_source/unittest_src_iio.cpp @@ -542,13 +542,18 @@ build_dev_dir_scan_elements (iio_dev_dir_struct * iio_dev, signchar = 's'; switch (idx % (iio_dev->num_scan_elements / 2)) { case 0: + /** big endian and signed */ break; case 1: + /** little endian and unsigned (missing break is intended) */ endianchar = 'l'; + /* fallthrough */ case 2: + /** big endian and unsigned */ signchar = 'u'; break; case 3: + /** little endian and signed */ endianchar = 'l'; break; } -- 2.7.4