From: Parichay Kapoor <pk.kapoor@samsung.com>
Date: Thu, 26 Sep 2019 03:03:23 +0000 (+0900)
Subject: [svace/iio] security fixes related to sscanf
X-Git-Tag: submit/tizen/20190930.043252~2
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=db96d34c0ecd9b5061781caf60dcb474a1ba73f2;p=platform%2Fupstream%2Fnnstreamer.git

[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 <pk.kapoor@samsung.com>
---

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;
     }