String8: fix infinite loop and segmentation fault in removeAll()
Bug: 290835996
Test: libutils_fuzz_string8 for several minutes
String8::removeAll() has 2 serious problems:
1. When `other` is an empty string, `removeAll()` will loop infinitely
due to below process:
a) with `other` being empty string `""`, find() will call strstr()
on an empty string, which always returns `mString`, and thus
find() always return 0 in this case
b) with find() returns 0 for empty string, the next while loop in
String8::removeAll() will keep loop infinitely as `index` will
always be 0
This CL fixes this problem by returning true if `other` is an empty
string (i.e. `strlen(other) == 0`), this follows the logic that an
empty string will always be found and no actual remove needs to be
done.
2. When `other` is a NULL string, strstr() has undefined behavior. See
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf.
This undefined behavior on Android unfortunately causes immediate
segmentation fault as the current `strstr` implementation in bionic
libc doesn't check `needle` being NULL, and an access to a NULL
location is performed to check if the `needle` string is an empty
string, and thus causes segmentation fault.
This CL gives an error message and aborts instead of having a
segfault, and to keep some backward compatibility.
This CL also adds test for String8::removeAll()
Change-Id: Ie2ccee6767efe0fed476db4ec6072717198279e9
diff --git a/libutils/String8.cpp b/libutils/String8.cpp
index 82f5cb6..8d312b5 100644
--- a/libutils/String8.cpp
+++ b/libutils/String8.cpp
@@ -393,6 +393,11 @@
}
bool String8::removeAll(const char* other) {
+ ALOG_ASSERT(other, "String8::removeAll() requires a non-NULL string");
+
+ if (*other == '\0')
+ return true;
+
ssize_t index = find(other);
if (index < 0) return false;