From 4d19713c7c113481124958284f7390b8bdc48e97 Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Tue, 28 Mar 2023 22:37:05 -0500 Subject: [PATCH] Security: Invalid read in trim comparison functions. 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 | 136 +++++++++++++++++----------------- 1 file changed, 69 insertions(+), 67 deletions(-) diff --git a/level_0/f_compare/c/private-compare.c b/level_0/f_compare/c/private-compare.c index 32e0bb7..c3f0407 100644 --- a/level_0/f_compare/c/private-compare.c +++ b/level_0/f_compare/c/private-compare.c @@ -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; } -- 1.8.3.1