ZNCString: CString::Replace and CString::Split loop forever on empty separator #1087

New Issue

2026-10-05T05:02:40+02:00

adam commented

2026-10-05 05:02:40 +02:00

`CString::Replace` and `CString::Split` loop forever on empty separator

Two sibling library-API bugs in CString. Same shape as #1994: a missing
input guard turns a degenerate argument into UB / unbounded loop. No
in-tree caller passes the bad argument today, but the public API is exposed
to module authors and to anyone embedding libznc.

1) `CString::Replace`: empty `sReplace` causes pointer underflow + OOM

// src/ZNCString.cpp
626: unsigned int CString::Replace(CString& sStr, const CString& sReplace,
627:                               const CString& sWith, const CString& sLeft,
628:                               const CString& sRight, bool bRemoveDelims) {
...
656:         } else if (!bInside &&
657:                    strncmp(p, sReplace.c_str(), uReplaceWidth) == 0) {
658:             sStr += sWith;
659:             p += uReplaceWidth - 1;        // <-- when uReplaceWidth==0: p -= 1
660:             uRet++;
...
665:         p++;                                // <-- back to where we started

When sReplace.empty(), strncmp(_, _, 0) == 0 always matches, the
arithmetic at 659 underflows size_t to SIZE_MAX, the p++ at 665
returns to the same byte, and the loop appends sWith indefinitely until
ASan kills the process for OOM.

PoC (3-second guard prevents the OOM kill):

#include <cstdio>
#include <signal.h>
#include <unistd.h>
#include <znc/ZNCString.h>
extern "C" void to(int) { _exit(124); }
int main() {
    signal(SIGALRM, to); alarm(3);
    CString s = "abc";
    CString::Replace(s, /*sReplace=*/"", /*sWith=*/"X");
}

ZNCString.cpp:659:15: runtime error: addition of unsigned offset to
  0x... overflowed to 0x...
[3s later] killed by SIGALRM

2) `CString::Split`: empty `sDelim` + `bAllowEmpty=false` infinite-loops

Same pattern, two locations:

// src/ZNCString.cpp
854:     if (!bAllowEmpty) {
855:         while (strncasecmp(p, sDelim.c_str(), uDelimLen) == 0) {
856:             p += uDelimLen;                // <-- 0 when sDelim.empty()
857:         }
858:     }
...
893:             if (!bAllowEmpty) {
894:                 while (strncasecmp(p, sDelim.c_str(), uDelimLen) == 0) {
895:                     p += uDelimLen;        // <-- same bug
896:                 }
897:             }

strncasecmp(_, _, 0) is unconditionally 0; p never advances; the loop
spins on the CPU forever. No allocation, so this hangs rather than OOMs.

PoC:

VCString out;
CString s = "abc";
s.Split(/*sDelim=*/"", out, /*bAllowEmpty=*/false);   // never returns

Reachability

I grepped every caller in src/ and modules/ and didn't find one that
passes an empty sReplace or empty sDelim. So this is the same
"library-API correctness, not currently reachable" framing as #1994. I'm
just bringing the contracts of these two functions in line with what
callers expect.

Fix

// in CString::Replace
if (sReplace.empty()) return 0;

// in CString::Split, guard the prefix-skip loop
if (!bAllowEmpty && !sDelim.empty()) {
    while (strncasecmp(p, sDelim.c_str(), uDelimLen) == 0) p += uDelimLen;
}
// and likewise at line 894.

Or, more aggressively, bail out at the top of Split if sDelim.empty().

Happy to PR with a couple of regression tests in test/StringTest.cpp
mirroring #1995.

# `CString::Replace` and `CString::Split` loop forever on empty separator Two sibling library-API bugs in `CString`. Same shape as #1994: a missing input guard turns a degenerate argument into UB / unbounded loop. No in-tree caller passes the bad argument today, but the public API is exposed to module authors and to anyone embedding libznc. ## 1) `CString::Replace`: empty `sReplace` causes pointer underflow + OOM ```cpp // src/ZNCString.cpp 626: unsigned int CString::Replace(CString& sStr, const CString& sReplace, 627: const CString& sWith, const CString& sLeft, 628: const CString& sRight, bool bRemoveDelims) { ... 656: } else if (!bInside && 657: strncmp(p, sReplace.c_str(), uReplaceWidth) == 0) { 658: sStr += sWith; 659: p += uReplaceWidth - 1; // <-- when uReplaceWidth==0: p -= 1 660: uRet++; ... 665: p++; // <-- back to where we started ``` When `sReplace.empty()`, `strncmp(_, _, 0) == 0` always matches, the arithmetic at 659 underflows `size_t` to `SIZE_MAX`, the `p++` at 665 returns to the same byte, and the loop appends `sWith` indefinitely until ASan kills the process for OOM. PoC (3-second guard prevents the OOM kill): ```cpp #include <cstdio> #include <signal.h> #include <unistd.h> #include <znc/ZNCString.h> extern "C" void to(int) { _exit(124); } int main() { signal(SIGALRM, to); alarm(3); CString s = "abc"; CString::Replace(s, /*sReplace=*/"", /*sWith=*/"X"); } ``` ``` ZNCString.cpp:659:15: runtime error: addition of unsigned offset to 0x... overflowed to 0x... [3s later] killed by SIGALRM ``` ## 2) `CString::Split`: empty `sDelim` + `bAllowEmpty=false` infinite-loops Same pattern, two locations: ```cpp // src/ZNCString.cpp 854: if (!bAllowEmpty) { 855: while (strncasecmp(p, sDelim.c_str(), uDelimLen) == 0) { 856: p += uDelimLen; // <-- 0 when sDelim.empty() 857: } 858: } ... 893: if (!bAllowEmpty) { 894: while (strncasecmp(p, sDelim.c_str(), uDelimLen) == 0) { 895: p += uDelimLen; // <-- same bug 896: } 897: } ``` `strncasecmp(_, _, 0)` is unconditionally 0; `p` never advances; the loop spins on the CPU forever. No allocation, so this hangs rather than OOMs. PoC: ```cpp VCString out; CString s = "abc"; s.Split(/*sDelim=*/"", out, /*bAllowEmpty=*/false); // never returns ``` ## Reachability I grepped every caller in `src/` and `modules/` and didn't find one that passes an empty `sReplace` or empty `sDelim`. So this is the same "library-API correctness, not currently reachable" framing as #1994. I'm just bringing the contracts of these two functions in line with what callers expect. ## Fix ```cpp // in CString::Replace if (sReplace.empty()) return 0; // in CString::Split, guard the prefix-skip loop if (!bAllowEmpty && !sDelim.empty()) { while (strncasecmp(p, sDelim.c_str(), uDelimLen) == 0) p += uDelimLen; } // and likewise at line 894. ``` Or, more aggressively, bail out at the top of `Split` if `sDelim.empty()`. Happy to PR with a couple of regression tests in `test/StringTest.cpp` mirroring #1995.

adam closed this issue

2026-10-05 05:02:40 +02:00

adam commented

2026-10-05 05:02:41 +02:00

Same as #2009 ?

Why you opening and closing the same issue as your other issues? And if you do, at least a link would be useful with explanation

Same as #2009 ? Why you opening and closing the same issue as your other issues? And if you do, at least a link would be useful with explanation

adam commented

2026-10-05 05:02:42 +02:00

Sorry, this was an accidental duplicate submission on my side. #2009 has the canonical content, please track that one and feel free to close this.

Sorry, this was an accidental duplicate submission on my side. #2009 has the canonical content, please track that one and feel free to close this.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: adam/znc#1087

ZNCString: CString::Replace and CString::Split loop forever on empty separator #1087

CString::Replace and CString::Split loop forever on empty separator

1) CString::Replace: empty sReplace causes pointer underflow + OOM

2) CString::Split: empty sDelim + bAllowEmpty=false infinite-loops

Reachability

Fix

`CString::Replace` and `CString::Split` loop forever on empty separator

1) `CString::Replace`: empty `sReplace` causes pointer underflow + OOM

2) `CString::Split`: empty `sDelim` + `bAllowEmpty=false` infinite-loops