From 512319eef887e7f9b5e1335bea354cd476f73f20 Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Tue, 28 Mar 2023 22:46:05 -0500 Subject: [PATCH] Security: Invalid read in trim comparison functions. This is a back port of the changes from the 0.7. development branch commit 4d19713c7c113481124958284f7390b8bdc48e97. The unit tests might also be back ported at a later time. 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_1/fl_string/c/private-string.c | 136 ++++++++++++++++++----------------- 1 file changed, 69 insertions(+), 67 deletions(-) diff --git a/level_1/fl_string/c/private-string.c b/level_1/fl_string/c/private-string.c index 3d1315d..73186d8 100644 --- a/level_1/fl_string/c/private-string.c +++ b/level_1/fl_string/c/private-string.c @@ -307,9 +307,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; } @@ -322,71 +320,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; } @@ -572,34 +572,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