diff --git a/src/ZNCString.cpp b/src/ZNCString.cpp index cf9c28ca..c30469b9 100644 --- a/src/ZNCString.cpp +++ b/src/ZNCString.cpp @@ -626,6 +626,14 @@ unsigned int CString::Replace(const CString& sReplace, const CString& sWith, unsigned int CString::Replace(CString& sStr, const CString& sReplace, const CString& sWith, const CString& sLeft, const CString& sRight, bool bRemoveDelims) { + // An empty needle would make strncmp(_, _, 0) match at every position + // and `p += uReplaceWidth - 1` underflow to SIZE_MAX, producing an + // infinite loop that appends sWith until OOM. Guard at the entry so + // the invariant "the loop always advances" holds. + if (sReplace.empty()) { + return 0; + } + unsigned int uRet = 0; CString sCopy = sStr; sStr.clear(); @@ -851,7 +859,10 @@ CString::size_type CString::Split(const CString& sDelim, VCString& vsRet, size_type uRightLen = sRight.length(); const char* p = c_str(); - if (!bAllowEmpty) { + // An empty delimiter with bAllowEmpty=false would spin forever in the + // prefix-skip / post-token loops below because `strncasecmp(_, _, 0)` + // returns 0 and `p += 0` never advances. + if (!bAllowEmpty && uDelimLen) { while (strncasecmp(p, sDelim.c_str(), uDelimLen) == 0) { p += uDelimLen; } @@ -890,7 +901,8 @@ CString::size_type CString::Split(const CString& sDelim, VCString& vsRet, sTmp.clear(); p += uDelimLen; - if (!bAllowEmpty) { + // Same zero-width guard as at the top of the function. + if (!bAllowEmpty && uDelimLen) { while (strncasecmp(p, sDelim.c_str(), uDelimLen) == 0) { p += uDelimLen; } diff --git a/test/StringTest.cpp b/test/StringTest.cpp index 4bde1b4c..9d40ecd9 100644 --- a/test/StringTest.cpp +++ b/test/StringTest.cpp @@ -14,9 +14,13 @@ * limitations under the License. */ +#include #include #include +using ::testing::ElementsAre; +using ::testing::IsEmpty; + class EscapeTest : public ::testing::Test { protected: void testEncode(const CString& in, const CString& expectedOut, @@ -128,6 +132,18 @@ TEST(StringTest, Replace) { EXPECT_EQ(CString("(a()a)").Replace_n("a", "b"), "(b()b)"); EXPECT_EQ(CString("(a()a)").Replace_n("a", "b", "(", ")"), "(a()b)"); EXPECT_EQ(CString("(a()a)").Replace_n("a", "b", "(", ")", true), "a(b)"); + + // An empty needle must return the input unchanged with a 0 count + // instead of looping forever appending sWith (#2009). + CString sStr = "abc"; + EXPECT_EQ(CString::Replace(sStr, "", "X"), 0u); + EXPECT_EQ(sStr, "abc"); + + sStr = ""; + EXPECT_EQ(CString::Replace(sStr, "", "X"), 0u); + EXPECT_EQ(sStr, ""); + + EXPECT_EQ(CString("abc").Replace_n("", "X"), "abc"); } TEST(StringTest, Misc) { @@ -172,6 +188,17 @@ TEST(StringTest, Split) { CS("a=x&c=d&a=b").URLSplit(mresult); EXPECT_EQ(mexpected, mresult) << "URLSplit"; + + // Empty delimiter must not spin in the prefix-skip loop (#2009). + // With nothing to split on, the whole input is returned as a single + // element (or zero elements if the input itself is empty). + VCString vempty; + EXPECT_EQ(CS("abc").Split("", vempty, false), 1u); + EXPECT_THAT(vempty, ElementsAre("abc")); + EXPECT_EQ(CS("abc").Split("", vempty, true), 1u); + EXPECT_THAT(vempty, ElementsAre("abc")); + EXPECT_EQ(CS("").Split("", vempty, false), 0u); + EXPECT_THAT(vempty, IsEmpty()); } TEST(StringTest, NamedFormat) {