]> Kevux Git Server - fll/commitdiff
Bugfix: Firewall length check from range is not calculating 0 correctly.
authorKevin Day <kevin@kevux.org>
Thu, 15 Feb 2024 04:47:49 +0000 (22:47 -0600)
committerKevin Day <kevin@kevux.org>
Thu, 15 Feb 2024 04:47:49 +0000 (22:47 -0600)
When the range.start is greater than the range.stop, then the length is 0.
Rather than checking for this, this just subtracts range.start from range.stop then adds 1.
This results in the case of say (0 - 1) + 1, which may not be 0 due to overflow behaviors.
Play it safe and explicitly test for this rather than hoping that the overflow operates ideally.

level_3/firewall/c/private-firewall.c

index fddef3660a569eea4cfb1da4f782db4b563b235b..3e05fa2d7b308b36047f75660401b745ed70529a 100644 (file)
@@ -74,7 +74,7 @@ f_status_t firewall_perform_commands(firewall_data_t * const data, firewall_loca
       data->main->signal_check = 0;
     }
 
-    length = (local->rule_objects.array[i].stop - local->rule_objects.array[i].start) + 1;
+    length = (local->rule_objects.array[i].start > local->rule_objects.array[i].stop) ? 0 : (local->rule_objects.array[i].stop - local->rule_objects.array[i].start) + 1;
     invalid = F_false;
 
     is_ip_list = F_false;
@@ -92,7 +92,7 @@ f_status_t firewall_perform_commands(firewall_data_t * const data, firewall_loca
         continue;
       }
 
-      length = (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
+      length = (local->rule_contents.array[i].array[0].start > local->rule_contents.array[i].array[0].stop) ? 0 : (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
 
       if (local->rule_contents.array[i].used != 1) {
         invalid = F_true;
@@ -124,7 +124,7 @@ f_status_t firewall_perform_commands(firewall_data_t * const data, firewall_loca
 
     // Process direction rule
     else if (fl_string_dynamic_compare_string(local->buffer.string + local->rule_objects.array[i].start, firewall_direction_s, length) == F_equal_to) {
-      length = (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
+      length = (local->rule_contents.array[i].array[0].start > local->rule_contents.array[i].array[0].stop) ? 0 : (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
 
       if (local->rule_contents.array[i].used != 1) {
         invalid = F_true;
@@ -149,7 +149,7 @@ f_status_t firewall_perform_commands(firewall_data_t * const data, firewall_loca
 
     // Process device rule.
     else if (fl_string_dynamic_compare_string(local->buffer.string + local->rule_objects.array[i].start, firewall_device_s, length) == F_equal_to) {
-      length = (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
+      length = (local->rule_contents.array[i].array[0].start > local->rule_contents.array[i].array[0].stop) ? 0 : (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
 
       if (local->rule_contents.array[i].used != 1) {
         invalid = F_true;
@@ -192,7 +192,7 @@ f_status_t firewall_perform_commands(firewall_data_t * const data, firewall_loca
 
     // Process action rule.
     else if (fl_string_dynamic_compare_string(local->buffer.string + local->rule_objects.array[i].start, firewall_action_s, length) == F_equal_to) {
-      length = (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
+      length = (local->rule_contents.array[i].array[0].start > local->rule_contents.array[i].array[0].stop) ? 0 : (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
 
       if (local->rule_contents.array[i].used != 1) {
         invalid = F_true;
@@ -218,7 +218,7 @@ f_status_t firewall_perform_commands(firewall_data_t * const data, firewall_loca
 
     // Process ip_list rule.
     else if (fl_string_dynamic_compare_string(local->buffer.string + local->rule_objects.array[i].start, firewall_ip_list, length) == F_equal_to) {
-      length = (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
+      length = (local->rule_contents.array[i].array[0].start > local->rule_contents.array[i].array[0].stop) ? 0 : (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
       is_ip_list = F_true;
 
       if (fl_string_dynamic_compare_string(local->buffer.string + local->rule_contents.array[i].array[0].start, firewall_ip_list_source_s, length) == F_equal_to) {
@@ -232,7 +232,7 @@ f_status_t firewall_perform_commands(firewall_data_t * const data, firewall_loca
       }
     }
     else if (fl_string_dynamic_compare_string(local->buffer.string + local->rule_objects.array[i].start, firewall_protocol_s, length) == F_equal_to) {
-      length = (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
+      length = (local->rule_contents.array[i].array[0].start > local->rule_contents.array[i].array[0].stop) ? 0 : (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
 
       if (local->rule_contents.array[i].used != 1) {
         invalid = F_true;
@@ -260,7 +260,7 @@ f_status_t firewall_perform_commands(firewall_data_t * const data, firewall_loca
 
     // Process tool rule.
     else if (fl_string_dynamic_compare_string(local->buffer.string + local->rule_objects.array[i].start, firewall_tool_s, length) == F_equal_to) {
-      length = (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
+      length = (local->rule_contents.array[i].array[0].start > local->rule_contents.array[i].array[0].stop) ? 0 : (local->rule_contents.array[i].array[0].stop - local->rule_contents.array[i].array[0].start) + 1;
 
       if (local->rule_contents.array[i].used != 1) {
         invalid = F_true;
@@ -308,7 +308,7 @@ f_status_t firewall_perform_commands(firewall_data_t * const data, firewall_loca
     }
 
     if (invalid) {
-      length = (local->rule_objects.array[i].stop - local->rule_objects.array[i].start) + 1;
+      length = (local->rule_objects.array[i].start > local->rule_objects.array[i].stop) ? 0 : (local->rule_objects.array[i].stop - local->rule_objects.array[i].start) + 1;
 
       if (length) {
         flockfile(data->main->warning.to.stream);
@@ -479,7 +479,7 @@ f_status_t firewall_perform_commands(firewall_data_t * const data, firewall_loca
           // Skip past the chain.
           ++subcounter;
 
-          length = (local->rule_contents.array[i].array[subcounter].stop - local->rule_contents.array[i].array[subcounter].start) + 1;
+          length = (local->rule_contents.array[i].array[subcounter].start > local->rule_contents.array[i].array[subcounter].stop) ? 0 : (local->rule_contents.array[i].array[subcounter].stop - local->rule_contents.array[i].array[subcounter].start) + 1;
 
           if (length) {
             ip_list.used = 0;
@@ -502,7 +502,7 @@ f_status_t firewall_perform_commands(firewall_data_t * const data, firewall_loca
 
         for (; subcounter < local->rule_contents.array[i].used; ++subcounter) {
 
-          length = (local->rule_contents.array[i].array[subcounter].stop - local->rule_contents.array[i].array[subcounter].start) + 1;
+          length = (local->rule_contents.array[i].array[subcounter].start > local->rule_contents.array[i].array[subcounter].stop) ? 0 : (local->rule_contents.array[i].array[subcounter].stop - local->rule_contents.array[i].array[subcounter].start) + 1;
 
           if (length) {
             arguments.array[arguments.used].used = 0;
@@ -515,7 +515,7 @@ f_status_t firewall_perform_commands(firewall_data_t * const data, firewall_loca
         } // for
       }
       else {
-        length = (local->rule_objects.array[i].stop - local->rule_objects.array[i].start) + 1;
+        length = (local->rule_objects.array[i].start > local->rule_objects.array[i].stop) ? 0 : (local->rule_objects.array[i].stop - local->rule_objects.array[i].start) + 1;
 
         flockfile(data->main->warning.to.stream);
 
@@ -872,7 +872,7 @@ f_status_t firewall_create_custom_chains(firewall_data_t * const data, firewall_
       if (F_status_is_error(status)) break;
 
       create_chain = F_true;
-      length = (local->chain_objects.array[i].stop - local->chain_objects.array[i].start) + 1;
+      length = (local->chain_objects.array[i].start > local->chain_objects.array[i].stop) ? 0 : (local->chain_objects.array[i].stop - local->chain_objects.array[i].start) + 1;
 
       arguments.array[1].used = 0;