From: Kevin Day <kevin@kevux.org>
Date: Thu, 26 Jan 2023 00:56:36 +0000 (-0600)
Subject: Security: NULL pointer dereference in writer due to pipe function result handling.
X-Git-Tag: 0.6.4~80
X-Git-Url: https://git.kevux.org/?a=commitdiff_plain;h=ce7d8e385d3518b752d262420bb477c4e2f0f549;p=fll

Security: NULL pointer dereference in writer due to pipe function result handling.

The GCC -fanalyzer parameter helped me discover this one.

The status_pipe is being read and processed.
There is a case where the status_pipe is being set but it is not being reset after handling.
In a later loop the pipe does not get read but the previously set state is used bringing the code into a bad state.
Then the loop doesn't do the block buffer used check and this results in the eventual NULL dereference.
---

diff --git a/level_3/fss_basic_list_write/c/private-write.c b/level_3/fss_basic_list_write/c/private-write.c
index 62ee9dd..324a241 100644
--- a/level_3/fss_basic_list_write/c/private-write.c
+++ b/level_3/fss_basic_list_write/c/private-write.c
@@ -193,6 +193,7 @@ extern "C" {
 
         range.start = 0;
         range.stop = block.used - 1;
+        status_pipe = F_none;
       }
 
       if (!state || state == 0x1) {
@@ -213,7 +214,7 @@ extern "C" {
           }
         }
 
-        for (; range.start <= range.stop; ++range.start) {
+        for (; range.start <= range.stop && range.start < block.used; ++range.start) {
 
           if (block.string[range.start] == fss_basic_list_write_pipe_content_start_s.string[0]) {
             state = 0x2;
@@ -264,7 +265,7 @@ extern "C" {
             break;
           }
 
-          for (; range.start <= range.stop; ++range.start) {
+          for (; range.start <= range.stop && range.start < block.used; ++range.start) {
 
             if (block.string[range.start] == fss_basic_list_write_pipe_content_start_s.string[0]) {
               if (main->error.verbosity != f_console_verbosity_quiet_e) {
diff --git a/level_3/fss_basic_write/c/private-write.c b/level_3/fss_basic_write/c/private-write.c
index 9c5c7f3..57983df 100644
--- a/level_3/fss_basic_write/c/private-write.c
+++ b/level_3/fss_basic_write/c/private-write.c
@@ -195,6 +195,7 @@ extern "C" {
 
         range.start = 0;
         range.stop = block.used - 1;
+        status_pipe = F_none;
       }
 
       if (!state || state == 0x1) {
@@ -215,7 +216,7 @@ extern "C" {
           }
         }
 
-        for (; range.start <= range.stop; range.start++) {
+        for (; range.start <= range.stop && range.start < block.used; range.start++) {
 
           if (block.string[range.start] == fss_basic_write_pipe_content_start_s.string[0]) {
             state = 0x2;
@@ -266,7 +267,7 @@ extern "C" {
             break;
           }
 
-          for (; range.start <= range.stop; ++range.start) {
+          for (; range.start <= range.stop && range.start < block.used; ++range.start) {
 
             if (block.string[range.start] == fss_basic_write_pipe_content_start_s.string[0]) {
               if (main->error.verbosity != f_console_verbosity_quiet_e) {
diff --git a/level_3/fss_embedded_list_write/c/private-write.c b/level_3/fss_embedded_list_write/c/private-write.c
index 2526241..8a73ea8 100644
--- a/level_3/fss_embedded_list_write/c/private-write.c
+++ b/level_3/fss_embedded_list_write/c/private-write.c
@@ -203,6 +203,7 @@ extern "C" {
 
         range.start = 0;
         range.stop = block.used - 1;
+        status_pipe = F_none;
       }
 
       if (!state || state == 0x1) {
@@ -226,7 +227,7 @@ extern "C" {
           }
         }
 
-        for (; range.start <= range.stop; ++range.start) {
+        for (; range.start <= range.stop && range.start < block.used; ++range.start) {
 
           if (block.string[range.start] == fss_embedded_list_write_pipe_content_start_s.string[0]) {
             state = 0x2;
@@ -243,6 +244,7 @@ extern "C" {
           }
 
           if (block.string[range.start] == fss_embedded_list_write_pipe_content_ignore_s.string[0]) {
+
             // This is not used by objects.
             continue;
           }
@@ -276,7 +278,7 @@ extern "C" {
             break;
           }
 
-          for (; range.start <= range.stop; ++range.start) {
+          for (; range.start <= range.stop && range.start < block.used; ++range.start) {
 
             if (block.string[range.start] == fss_embedded_list_write_pipe_content_start_s.string[0]) {
               if (main->error.verbosity != f_console_verbosity_quiet_e) {
diff --git a/level_3/fss_extended_list_write/c/private-write.c b/level_3/fss_extended_list_write/c/private-write.c
index 47e754f..b6ff627 100644
--- a/level_3/fss_extended_list_write/c/private-write.c
+++ b/level_3/fss_extended_list_write/c/private-write.c
@@ -193,6 +193,7 @@ extern "C" {
 
         range.start = 0;
         range.stop = block.used - 1;
+        status_pipe = F_none;
       }
 
       if (!state || state == 0x1) {
@@ -216,7 +217,7 @@ extern "C" {
           }
         }
 
-        for (; range.start <= range.stop; ++range.start) {
+        for (; range.start <= range.stop && range.start < block.used; ++range.start) {
 
           if (block.string[range.start] == fss_extended_list_write_pipe_content_start_s.string[0]) {
             state = 0x2;
@@ -267,7 +268,7 @@ extern "C" {
             break;
           }
 
-          for (; range.start <= range.stop; ++range.start) {
+          for (; range.start <= range.stop && range.start < block.used; ++range.start) {
 
             if (block.string[range.start] == fss_extended_list_write_pipe_content_start_s.string[0]) {
               if (main->error.verbosity != f_console_verbosity_quiet_e) {
diff --git a/level_3/fss_extended_write/c/private-write.c b/level_3/fss_extended_write/c/private-write.c
index e96c26b..936ea2f 100644
--- a/level_3/fss_extended_write/c/private-write.c
+++ b/level_3/fss_extended_write/c/private-write.c
@@ -201,6 +201,7 @@ extern "C" {
           fll_error_print(main->error, F_status_set_fine(status_pipe), "f_file_read_block", F_true);
 
           status_pipe = F_status_set_error(F_pipe);
+
           break;
         }
 
@@ -212,28 +213,24 @@ extern "C" {
 
       if (!state || state == 0x1) {
         if (!state) {
-          if (contents.used) {
-            for (i = 0; i < contents.used; ++i) {
-              contents.array[i].used = 0;
-            } // for
-          }
+          for (i = 0; i < contents.used; ++i) {
+            contents.array[i].used = 0;
+          } // for
 
           object.used = 0;
           contents.used = 0;
-
           state = 0x1;
         }
 
-        if (object.used + block.used > object.size) {
-          status = f_string_dynamic_increase_by(block.used, &object);
+        status = f_string_dynamic_increase_by(block.used, &object);
 
-          if (F_status_is_error(status)) {
-            fll_error_print(main->error, F_status_set_fine(status), "f_string_dynamic_increase_by", F_true);
-            break;
-          }
+        if (F_status_is_error(status)) {
+          fll_error_print(main->error, F_status_set_fine(status), "f_string_dynamic_increase_by", F_true);
+
+          break;
         }
 
-        for (; range.start <= range.stop; ++range.start) {
+        for (; range.start <= range.stop && range.start < block.used; ++range.start) {
 
           if (block.string[range.start] == fss_extended_write_pipe_content_start_s.string[0]) {
             state = 0x2;
@@ -268,13 +265,12 @@ extern "C" {
       }
 
       if (state == 0x2) {
-        if (contents.used + 1 > contents.size) {
-          status = f_string_dynamics_increase_by(F_fss_default_allocation_step_d, &contents);
+        status = f_string_dynamics_increase_by(F_fss_default_allocation_step_d, &contents);
 
-          if (F_status_is_error(status)) {
-            fll_error_print(main->error, F_status_set_fine(status), "f_string_dynamics_increase_by", F_true);
-            break;
-          }
+        if (F_status_is_error(status)) {
+          fll_error_print(main->error, F_status_set_fine(status), "f_string_dynamics_increase_by", F_true);
+
+          break;
         }
 
         state = 0x3;
@@ -290,17 +286,15 @@ extern "C" {
         }
 
         if (total) {
-          for (; range.start <= range.stop; ++range.start) {
+          for (; range.start <= range.stop && range.start < block.used; ++range.start) {
 
             if (block.string[range.start] == fss_extended_write_pipe_content_start_s.string[0]) {
-              if (contents.used + 1 > contents.size) {
-                status = f_string_dynamics_increase_by(F_fss_default_allocation_step_d, &contents);
+              status = f_string_dynamics_increase_by(F_fss_default_allocation_step_d, &contents);
 
-                if (F_status_is_error(status)) {
-                  fll_error_print(main->error, F_status_set_fine(status), "f_string_dynamics_increase_by", F_true);
+              if (F_status_is_error(status)) {
+                fll_error_print(main->error, F_status_set_fine(status), "f_string_dynamics_increase_by", F_true);
 
-                  break;
-                }
+                break;
               }
 
               ++contents.used;
@@ -329,14 +323,12 @@ extern "C" {
               break;
             }
 
-            if (contents.array[contents.used - 1].used + 1 > contents.array[contents.used - 1].size) {
-              status = f_string_dynamic_increase_by(F_fss_default_allocation_step_d, &contents.array[contents.used - 1]);
+            status = f_string_dynamic_increase_by(F_fss_default_allocation_step_d, &contents.array[contents.used - 1]);
 
-              if (F_status_is_error(status)) {
-                fll_error_print(main->error, F_status_set_fine(status), "f_string_dynamic_increase_by", F_true);
+            if (F_status_is_error(status)) {
+              fll_error_print(main->error, F_status_set_fine(status), "f_string_dynamic_increase_by", F_true);
 
-                break;
-              }
+              break;
             }
 
             contents.array[contents.used - 1].string[contents.array[contents.used - 1].used++] = block.string[range.start];
diff --git a/level_3/fss_payload_write/c/private-write.c b/level_3/fss_payload_write/c/private-write.c
index d658f27..6c656cf 100644
--- a/level_3/fss_payload_write/c/private-write.c
+++ b/level_3/fss_payload_write/c/private-write.c
@@ -240,6 +240,7 @@ extern "C" {
 
         range.start = 0;
         range.stop = block.used - 1;
+        status_pipe = F_none;
       }
 
       if (!state || state == 0x1) {
@@ -260,7 +261,7 @@ extern "C" {
           }
         }
 
-        for (; range.start <= range.stop; ++range.start) {
+        for (; range.start <= range.stop && range.start < block.used; ++range.start) {
 
           if (block.string[range.start] == fss_payload_write_pipe_content_start_s.string[0]) {
             state = 0x2;
@@ -336,7 +337,7 @@ extern "C" {
             break;
           }
 
-          for (; range.start <= range.stop; ++range.start) {
+          for (; range.start <= range.stop && range.start < block.used; ++range.start) {
 
             if (block.string[range.start] == fss_payload_write_pipe_content_start_s.string[0]) {
               if (main->error.verbosity != f_console_verbosity_quiet_e) {