Fix undefined behavior on empty input in strinc() and prefixRange()
CI / pre-commit (pull_request) Successful in 2m5s
CI / release (arm64, ubuntu-latest-arm64) (pull_request) Successful in 3m29s
CI / test (-DCMAKE_BUILD_TYPE=Debug, debug) (pull_request) Successful in 3m34s
CI / test (-DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 64-bit-versions) (pull_request) Successful in 3m28s
CI / test (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc) (pull_request) Successful in 3m38s
CI / test (-DUSE_SIMD_FALLBACK=ON, simd-fallback) (pull_request) Successful in 3m27s
CI / release (amd64, ubuntu-latest-amd64) (pull_request) Successful in 4m51s
CI / coverage (pull_request) Successful in 3m37s
CI / pre-commit (pull_request) Successful in 2m5s
CI / release (arm64, ubuntu-latest-arm64) (pull_request) Successful in 3m29s
CI / test (-DCMAKE_BUILD_TYPE=Debug, debug) (pull_request) Successful in 3m34s
CI / test (-DCMAKE_CXX_FLAGS=-DUSE_64_BIT=1, 64-bit-versions) (pull_request) Successful in 3m28s
CI / test (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc) (pull_request) Successful in 3m38s
CI / test (-DUSE_SIMD_FALLBACK=ON, simd-fallback) (pull_request) Successful in 3m27s
CI / release (amd64, ubuntu-latest-amd64) (pull_request) Successful in 4m51s
CI / coverage (pull_request) Successful in 3m37s
strinc() in ConflictSet.cpp used std::string_view::size() (size_t) and subtracted 1 without first checking for an empty string. For the root node, getSearchPath() returns the empty string, so every debug correctness check underflowed size_t and relied on implementation-defined conversion to signed int. prefixRange() in Bench.cpp had the same loop shape. Although TrivialSpan::size() returns int, on an empty (or all-0xff) key the function then asserted and continued executing, allocating a zero-length buffer and writing before its start. Changes: - In strinc(), initialize index as signed int(str.size()) - 1 so the loop is skipped for empty input, and return ok=false cleanly. - In prefixRange(), initialize index the same way and call std::abort() after the assert so invalid input cannot fall through to an out-of-bounds write. - Replace C-style uint8_t casts with explicit static_casts. Closes #59
This commit is contained in:
@@ -37,15 +37,16 @@ ConflictSet::ReadRange singleton(Arena &arena, TrivialSpan key) {
|
||||
}
|
||||
|
||||
ConflictSet::ReadRange prefixRange(Arena &arena, TrivialSpan key) {
|
||||
int index;
|
||||
for (index = key.size() - 1; index >= 0; index--)
|
||||
if ((key[index]) != 255)
|
||||
int index = key.size() - 1;
|
||||
for (; index >= 0; index--)
|
||||
if (key[index] != 255)
|
||||
break;
|
||||
|
||||
// Must not be called with a string that consists only of zero or more '\xff'
|
||||
// bytes.
|
||||
// bytes, or with an empty string (which has no finite upper bound).
|
||||
if (index < 0) {
|
||||
assert(false);
|
||||
std::abort();
|
||||
}
|
||||
|
||||
uint8_t *buf = new (arena) uint8_t[index + 1];
|
||||
|
||||
+6
-5
@@ -5679,13 +5679,13 @@ std::string getPartialKeyPrintable(Node *n) {
|
||||
}
|
||||
|
||||
std::string strinc(std::string_view str, bool &ok) {
|
||||
int index;
|
||||
for (index = str.size() - 1; index >= 0; index--)
|
||||
if ((uint8_t &)(str[index]) != 255)
|
||||
int index = static_cast<int>(str.size()) - 1;
|
||||
for (; index >= 0; index--)
|
||||
if (static_cast<uint8_t>(str[index]) != 255)
|
||||
break;
|
||||
|
||||
// Must not be called with a string that consists only of zero or more
|
||||
// '\xff' bytes.
|
||||
// '\xff' bytes, and the empty string has no successor.
|
||||
if (index < 0) {
|
||||
ok = false;
|
||||
return {};
|
||||
@@ -5693,7 +5693,8 @@ std::string strinc(std::string_view str, bool &ok) {
|
||||
ok = true;
|
||||
|
||||
auto r = std::string(str.substr(0, index + 1));
|
||||
((uint8_t &)r[r.size() - 1])++;
|
||||
auto &last = r[r.size() - 1];
|
||||
last = static_cast<char>(static_cast<uint8_t>(last) + 1);
|
||||
return r;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user