From: Kevin Day <thekevinday@gmail.com>
Date: Sat, 7 Aug 2021 04:48:55 +0000 (-0500)
Subject: Security: NULL dereferences discovered by GCC's -fanalyzer.
X-Git-Tag: 0.5.5~18
X-Git-Url: https://git.kevux.org/?a=commitdiff_plain;h=d3a65ad8c5606842fa63b79a47a4fb6b5aec425e;p=fll

Security: NULL dereferences discovered by GCC's -fanalyzer.

The socket bind is both missing the parameter check and is passing '&' when the variable is already a pointer.

The f_utf_character_to_char() is a mess.
Not sure what I was trying to do, but it is clearly wrong.
Redesign it to be more correct but I should revisit this for a more thorough review.
There are endianess situations that also need resolved so I added an @todo.
---

diff --git a/level_0/f_socket/c/socket.c b/level_0/f_socket/c/socket.c
index af2863a..8bd42af 100644
--- a/level_0/f_socket/c/socket.c
+++ b/level_0/f_socket/c/socket.c
@@ -6,8 +6,11 @@ extern "C"{
 
 #ifndef _di_f_socket_file_bind_
   f_status_t f_socket_file_bind(const f_string_t path, const int id, struct sockaddr_un *address) {
+    #ifndef _di_level_0_parameter_checking_
+      if (!address) return F_status_set_error(F_parameter);
+    #endif // _di_level_0_parameter_checking_
 
-    memset(&address, 0, sizeof(struct sockaddr_un));
+    memset(address, 0, sizeof(struct sockaddr_un));
     address->sun_family = AF_UNIX;
 
     strncpy(address->sun_path, path, sizeof(address->sun_path) - 1);
diff --git a/level_0/f_socket/c/socket.h b/level_0/f_socket/c/socket.h
index a1008ac..6664939 100644
--- a/level_0/f_socket/c/socket.h
+++ b/level_0/f_socket/c/socket.h
@@ -53,6 +53,7 @@ extern "C"{
  *   F_file_found_not (with error bit) if file not found.
  *   F_memory_not (with error bit) if out of memory.
  *   F_name (with error bit) on path name error.
+ *   F_parameter (with error bit) if a parameter is invalid.
  *   F_string_too_large (with error bit) if string is too large to store in the buffer.
  *   F_available_not_address (with error bit) if address is unavailable (is non-existent or not local).
  *   F_failure (with error bit) for any other error.
diff --git a/level_0/f_utf/c/utf.c b/level_0/f_utf/c/utf.c
index 7d55a57..d9c6ef2 100644
--- a/level_0/f_utf/c/utf.c
+++ b/level_0/f_utf/c/utf.c
@@ -662,52 +662,51 @@ extern "C" {
   f_status_t f_utf_character_to_char(const f_utf_character_t utf_character, f_string_t *character, f_array_length_t *width_max) {
     #ifndef _di_level_0_parameter_checking_
       if (!utf_character) return F_status_set_error(F_parameter);
-      if (!width_max && *character) return F_status_set_error(F_parameter);
-      if (width_max && !*character) return F_status_set_error(F_parameter);
-      if (width_max && *width_max > 4) return F_status_set_error(F_parameter);
+      if (!character) return F_status_set_error(F_parameter);
+      if (!width_max) return F_status_set_error(F_parameter);
+      if (!*width_max) return F_status_set_error(F_parameter);
     #endif // _di_level_0_parameter_checking_
 
-    f_status_t status = F_none;
-
-    uint8_t width = macro_f_utf_character_t_width_is(utf_character);
-
-    if (!width_max) {
-      macro_f_string_t_clear((*character))
-      macro_f_string_t_resize(status, (*character), 0, width)
-      if (F_status_is_error(status)) return status;
-
-      width = 1;
-      *width_max = 1;
-    }
-    else if (width == 1) {
-      return F_status_is_error(F_utf);
-    }
-    else if (width > *width_max) {
-      return F_status_set_error(F_failure);
-    }
-
-    *width_max = width;
+    if (macro_f_utf_character_t_width_is(utf_character)) {
+
+      // @todo: endianess is compile time so a function is not needed, replace with macros.
+      if (f_utf_is_big_endian()) {
+        memcpy(*character, &utf_character, macro_f_utf_character_t_width_is(utf_character));
+      }
+      else {
+        uint32_t utf = 0;
+
+        switch (macro_f_utf_character_t_width_is(utf_character)) {
+          case 1:
+            utf = macro_f_utf_character_t_to_char_1(utf_character) << 24;
+            break;
+          case 2:
+            utf = (macro_f_utf_character_t_to_char_2(utf_character) << 24) | (macro_f_utf_character_t_to_char_1(utf_character) << 16);
+            break;
+          case 3:
+            utf = (macro_f_utf_character_t_to_char_3(utf_character) << 24) | (macro_f_utf_character_t_to_char_2(utf_character) << 16) | (macro_f_utf_character_t_to_char_1(utf_character) << 8);
+            break;
+          case 4:
+            utf = (macro_f_utf_character_t_to_char_4(utf_character) << 24) | (macro_f_utf_character_t_to_char_3(utf_character) << 16) | (macro_f_utf_character_t_to_char_2(utf_character) << 8) | macro_f_utf_character_t_to_char_1(utf_character);
+            break;
+          default:
+            return F_status_set_error(F_failure);
+        }
 
-    if (f_utf_is_big_endian()) {
-      memcpy(*character, &utf_character, width);
+        memcpy(*character, &utf, macro_f_utf_character_t_width_is(utf_character));
+      }
     }
     else {
-      uint32_t utf = 0;
 
-      if (width == 1) {
-        utf = macro_f_utf_character_t_to_char_1(utf_character) << 24;
-      }
-      else if (width == 2) {
-        utf = (macro_f_utf_character_t_to_char_2(utf_character) << 24) | (macro_f_utf_character_t_to_char_1(utf_character) << 16);
-      }
-      else if (width == 3) {
-        utf = (macro_f_utf_character_t_to_char_3(utf_character) << 24) | (macro_f_utf_character_t_to_char_2(utf_character) << 16) | (macro_f_utf_character_t_to_char_1(utf_character) << 8);
-      }
-      else if (width == 4) {
-        utf = (macro_f_utf_character_t_to_char_4(utf_character) << 24) | (macro_f_utf_character_t_to_char_3(utf_character) << 16) | (macro_f_utf_character_t_to_char_2(utf_character) << 8) | macro_f_utf_character_t_to_char_1(utf_character);
+      // @todo: endianess is compile time so a function is not needed, replace with macros.
+      if (f_utf_is_big_endian()) {
+        memcpy(*character, &utf_character, 1);
       }
+      else {
+        uint32_t utf = macro_f_utf_character_t_to_char_1(utf_character) << 24;
 
-      memcpy(*character, &utf, width);
+        memcpy(*character, &utf, 1);
+      }
     }
 
     return F_none;
diff --git a/level_0/f_utf/c/utf.h b/level_0/f_utf/c/utf.h
index 059c345..5d95774 100644
--- a/level_0/f_utf/c/utf.h
+++ b/level_0/f_utf/c/utf.h
@@ -669,26 +669,23 @@ extern "C" {
  * Convert a specialized f_utf_character_t type to a uint8_t, stored as a string (character buffer).
  *
  * This will also convert ASCII characters stored in the utf_character array.
+ * This will not resize character.
  *
  * @param utf_character
- *   The UTF-8 characterr to convert from.
+ *   The UTF-8 character to convert from.
  * @param character
  *   A uint8_t representation of the UTF-8 character, stored as a string of width bytes.
- *   If width_max is 0, then this should not be allocated (set the pointer address to 0).
+ *   If width_max is 0, then this should be set to 0.
  * @param width_max
- *   The number of bytes the generated character represents.
- *   If this is set to 0, then the character will be allocated and this will be set to the width of the utf_character.
- *   If this is set to some value greater than 0 (up to 4), then this represents the size of the character array (no allocations are performed).
- *   If this is greater than 0, and the utf_character width is larger than this size, then an error is returned.
+ *   This is set to the max number of bytes available.
+ *   This is then updated to represent the max bytes used if enough space is available.
  *
  * @return
  *   F_none if conversion was successful.
  *
  *   F_failure (with error bit) if width is not long enough to convert.
  *   F_parameter (with error bit) if a parameter is invalid.
- *   F_memory_not (with error bit) on out of memory.
  *   F_utf (with error bit) if character is an invalid UTF-8 character.
- *   F_failure (with error bit) if width is not long enough to convert.
  */
 #ifndef _di_f_utf_character_to_char_
   extern f_status_t f_utf_character_to_char(const f_utf_character_t utf_character, f_string_t *character, f_array_length_t *width_max);