From 04cf89beec2e933adaf7ae1f5afd043985e16aa9 Mon Sep 17 00:00:00 2001 From: MarkLee131 Date: Sat, 25 Apr 2026 10:38:31 +0800 Subject: [PATCH 1/2] HTTPSock: reject CR/LF in AddHeader name/value AddHeader wrote its arguments straight into the response stream. No in-tree caller reaches it with attacker-controlled bytes today, but the public API is exposed to module authors; one bad caller would be a header-injection bug. Filter at the entry rather than at every caller. --- src/HTTPSock.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/HTTPSock.cpp b/src/HTTPSock.cpp index 237713e6..0a4e48f7 100644 --- a/src/HTTPSock.cpp +++ b/src/HTTPSock.cpp @@ -763,6 +763,13 @@ void CHTTPSock::SetContentType(const CString& sContentType) { } void CHTTPSock::AddHeader(const CString& sName, const CString& sValue) { + // Reject CR/LF in either half so we never emit a malformed header or + // give a caller (e.g. a future module) a cheap response-splitting + // primitive. No in-tree caller reaches this with attacker-controlled + // bytes today; this is a defensive guard, not a fix for an existing + // exploit. + if (sName.find_first_of("\r\n") != CString::npos) return; + if (sValue.find_first_of("\r\n") != CString::npos) return; m_msHeaders[sName] = sValue; } From 20e8f73b034130128d91dcb6016a6c6f3906d6f1 Mon Sep 17 00:00:00 2001 From: MarkLee131 Date: Sat, 25 Apr 2026 17:38:31 +0800 Subject: [PATCH 2/2] HTTPSock: extract IsValidHeaderField helper and add tests (#2010) --- include/znc/HTTPSock.h | 4 ++++ src/HTTPSock.cpp | 7 +++++-- test/CMakeLists.txt | 2 +- test/HTTPSockTest.cpp | 39 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 test/HTTPSockTest.cpp diff --git a/include/znc/HTTPSock.h b/include/znc/HTTPSock.h index ca3e737a..d2b2edd4 100644 --- a/include/znc/HTTPSock.h +++ b/include/znc/HTTPSock.h @@ -52,6 +52,10 @@ class CHTTPSock : public CSocket { unsigned int uStatusId = 200, const CString& sStatusMsg = "OK"); void AddHeader(const CString& sName, const CString& sValue); + /** Returns false if `s` contains CR or LF. Used by AddHeader to reject + * inputs that could split the response into a separate header or body + * (RFC 7230 disallows obs-fold; treat any bare CR/LF as invalid). */ + static bool IsValidHeaderField(const CString& s); void SetContentType(const CString& sContentType); bool PrintNotFound(); diff --git a/src/HTTPSock.cpp b/src/HTTPSock.cpp index 0a4e48f7..2ff3a643 100644 --- a/src/HTTPSock.cpp +++ b/src/HTTPSock.cpp @@ -762,14 +762,17 @@ void CHTTPSock::SetContentType(const CString& sContentType) { m_sContentType = sContentType; } +bool CHTTPSock::IsValidHeaderField(const CString& s) { + return s.find_first_of("\r\n") == CString::npos; +} + void CHTTPSock::AddHeader(const CString& sName, const CString& sValue) { // Reject CR/LF in either half so we never emit a malformed header or // give a caller (e.g. a future module) a cheap response-splitting // primitive. No in-tree caller reaches this with attacker-controlled // bytes today; this is a defensive guard, not a fix for an existing // exploit. - if (sName.find_first_of("\r\n") != CString::npos) return; - if (sValue.find_first_of("\r\n") != CString::npos) return; + if (!IsValidHeaderField(sName) || !IsValidHeaderField(sValue)) return; m_msHeaders[sName] = sValue; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 92f4f621..f928dc83 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -61,7 +61,7 @@ add_executable(unittest_bin EXCLUDE_FROM_ALL "ThreadTest.cpp" "NickTest.cpp" "ClientTest.cpp" "NetworkTest.cpp" "MessageTest.cpp" "ModulesTest.cpp" "IRCSockTest.cpp" "QueryTest.cpp" "StringTest.cpp" "ConfigTest.cpp" "BufferTest.cpp" "UtilsTest.cpp" - "UserTest.cpp" "DebugTest.cpp") + "UserTest.cpp" "DebugTest.cpp" "HTTPSockTest.cpp") target_link_libraries(unittest_bin PRIVATE znclib) target_include_directories(unittest_bin PRIVATE "${GTEST_ROOT}" "${GTEST_ROOT}/include" diff --git a/test/HTTPSockTest.cpp b/test/HTTPSockTest.cpp new file mode 100644 index 00000000..420d847b --- /dev/null +++ b/test/HTTPSockTest.cpp @@ -0,0 +1,39 @@ +/* + * Copyright (C) 2004-2026 ZNC, see the NOTICE file for details. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +// Validation contract used by AddHeader to keep CR/LF (and therefore +// response-splitting bytes) out of the response stream (#2010). +TEST(HTTPSockTest, IsValidHeaderField) { + // Plain field names and values are accepted. + EXPECT_TRUE(CHTTPSock::IsValidHeaderField("")); + EXPECT_TRUE(CHTTPSock::IsValidHeaderField("X-Custom")); + EXPECT_TRUE(CHTTPSock::IsValidHeaderField("text/html; charset=utf-8")); + EXPECT_TRUE(CHTTPSock::IsValidHeaderField("a value with spaces and tabs\t")); + + // CR or LF anywhere is rejected; both halves of a CRLF pair are + // rejected even individually. + EXPECT_FALSE(CHTTPSock::IsValidHeaderField("\r")); + EXPECT_FALSE(CHTTPSock::IsValidHeaderField("\n")); + EXPECT_FALSE(CHTTPSock::IsValidHeaderField("\r\n")); + EXPECT_FALSE(CHTTPSock::IsValidHeaderField("X\rFoo")); + EXPECT_FALSE(CHTTPSock::IsValidHeaderField("X\nFoo")); + EXPECT_FALSE(CHTTPSock::IsValidHeaderField("safe\r\nInjected: yes")); + EXPECT_FALSE(CHTTPSock::IsValidHeaderField("trailing\n")); + EXPECT_FALSE(CHTTPSock::IsValidHeaderField("\rleading")); +}