]> Kevux Git Server - fll/commitdiff
Security: Invalid read in trim comparison functions.
authorKevin Day <kevin@kevux.org>
Wed, 29 Mar 2023 03:37:05 +0000 (22:37 -0500)
committerKevin Day <kevin@kevux.org>
Wed, 29 Mar 2023 03:37:05 +0000 (22:37 -0500)
Writing unit tests exposed this problem.

The last1 and last2 positions could be the exclusive stop points.
The comparison checks fail to handle this situation and expect the last1 and last2 variables to not be positioned at an exclusive stop point.
This results in an invalid read.

level_0/f_compare/c/private-compare.c

index 32e0bb794c4da1917c74fff3c8526f1b1e66a100..c3f04079d9d4b90549fea520e3bfcd8007e956e7 100644 (file)
@@ -301,9 +301,7 @@ extern "C" {
         status = f_utf_is_whitespace(string2 + j, width_max, F_false);
 
         if (F_status_is_error(status)) {
-          if (F_status_set_fine(status) == F_maybe) {
-            return F_status_set_error(F_utf_not);
-          }
+          if (F_status_set_fine(status) == F_maybe) return F_status_set_error(F_utf_not);
 
           return status;
         }
@@ -316,71 +314,73 @@ extern "C" {
         }
       } // for
 
-      if (size1 != size2) {
-        return F_equal_to_not;
-      }
+      if (size1 != size2) return F_equal_to_not;
     }
 
-    while (i1 <= last1 && i2 <= last2) {
+    if (last1 < stop1 && last2 < stop2) {
+      while (i1 <= last1 && i2 <= last2) {
 
-      // Skip past NULL in string1.
-      while (i1 <= last1 && !string1[i1]) ++i1;
-      if (i1 > last1) break;
+        // Skip past NULL in string1.
+        while (i1 <= last1 && !string1[i1]) ++i1;
+        if (i1 > last1) break;
 
-      // Skip past NULL in string2.
-      while (i2 <= last2 && !string2[i2]) ++i2;
-      if (i2 > last2) break;
+        // Skip past NULL in string2.
+        while (i2 <= last2 && !string2[i2]) ++i2;
+        if (i2 > last2) break;
 
-      // Skip past except characters in string1.
-      while (e1 < except1.used && except1.array[e1] < i1) ++e1;
+        // Skip past except characters in string1.
+        while (e1 < except1.used && except1.array[e1] < i1) ++e1;
 
-      if (e1 < except1.used && except1.array[e1] == i1) {
-        ++i1;
+        if (e1 < except1.used && except1.array[e1] == i1) {
+          ++i1;
 
-        continue;
-      }
+          continue;
+        }
 
-      // Skip past except characters in string2.
-      while (e2 < except2.used && except2.array[e2] < i2) ++e2;
+        // Skip past except characters in string2.
+        while (e2 < except2.used && except2.array[e2] < i2) ++e2;
 
-      if (e2 < except2.used && except2.array[e2] == i2) {
-        ++i2;
+        if (e2 < except2.used && except2.array[e2] == i2) {
+          ++i2;
 
-        continue;
-      }
+          continue;
+        }
 
-      if (string1[i1] != string2[i2]) {
-        return F_equal_to_not;
-      }
+        if (string1[i1] != string2[i2]) return F_equal_to_not;
 
-      ++i1;
-      ++i2;
-    } // while
+        ++i1;
+        ++i2;
+      } // while
+    }
 
     // Only return F_equal_to if all remaining characters are NULL.
-    for (; i1 <= last1; ++i1) {
+    if (last1 < stop1) {
+      for (; i1 <= last1; ++i1) {
 
-      if (string1[i1] != 0) {
+        if (string1[i1] != 0) {
 
-        // Skip past except characters in string1.
-        while (e1 < except1.used && except1.array[e1] < i1) ++e1;
-        if (e1 < except1.used && except1.array[e1] == i1) continue;
+          // Skip past except characters in string1.
+          while (e1 < except1.used && except1.array[e1] < i1) ++e1;
+          if (e1 < except1.used && except1.array[e1] == i1) continue;
 
-        return F_equal_to_not;
-      }
-    } // for
+          return F_equal_to_not;
+        }
+      } // for
+    }
 
-    for (; i2 <= last2; ++i2) {
+    if (last2 < stop2) {
+      for (; i2 <= last2; ++i2) {
 
-      if (string2[i2] != 0) {
+        if (string2[i2] != 0) {
 
-        // Skip past except characters in string1.
-        while (e2 < except2.used && except2.array[e2] < i2) ++e2;
-        if (e2 < except2.used && except2.array[e2] == i2) continue;
+          // Skip past except characters in string1.
+          while (e2 < except2.used && except2.array[e2] < i2) ++e2;
+          if (e2 < except2.used && except2.array[e2] == i2) continue;
 
-        return F_equal_to_not;
-      }
-    } // for
+          return F_equal_to_not;
+        }
+      } // for
+    }
 
     return F_equal_to;
   }
@@ -566,34 +566,36 @@ extern "C" {
         }
       } // for
 
-      if (size1 != size2) {
-        return F_equal_to_not;
-      }
+      if (size1 != size2) return F_equal_to_not;
     }
 
-    for (; i1 < last1 && i2 < last2; ++i1, ++i2) {
+    if (last1 < stop1 && last2 < stop2) {
+      for (; i1 < last1 && i2 < last2; ++i1, ++i2) {
 
-      // Skip past NULL in string1.
-      while (i1 < last1 && !string1[i1]) ++i1;
-      if (i1 == last1) break;
+        // Skip past NULL in string1.
+        while (i1 < last1 && !string1[i1]) ++i1;
+        if (i1 == last1) break;
 
-      // Skip past NULL in string2.
-      while (i2 < last2 && !string2[i2]) ++i2;
-      if (i2 == last2) break;
+        // Skip past NULL in string2.
+        while (i2 < last2 && !string2[i2]) ++i2;
+        if (i2 == last2) break;
 
-      if (string1[i1] != string2[i2]) {
-        return F_equal_to_not;
-      }
-    } // for
+        if (string1[i1] != string2[i2]) return F_equal_to_not;
+      } // for
+    }
 
     // Only return F_equal_to if all remaining characters are NULL.
-    for (; i1 < last1; ++i1) {
-      if (string1[i1] != 0) return F_equal_to_not;
-    } // for
+    if (last1 < stop1) {
+      for (; i1 < last1; ++i1) {
+        if (string1[i1] != 0) return F_equal_to_not;
+      } // for
+    }
 
-    for (; i2 < last2; ++i2) {
-      if (string2[i2] != 0) return F_equal_to_not;
-    } // for
+    if (last2 < stop2) {
+      for (; i2 < last2; ++i2) {
+        if (string2[i2] != 0) return F_equal_to_not;
+      } // for
+    }
 
     return F_equal_to;
   }