fix: bound HTTP camera response header copies to prevent buffer overflow

RemoteCameraHTTP::GetResponse()'s non-PCRE parser (the default when built
without PCRE) copied server-controlled HTTP response header values into
fixed-size static buffers with unbounded strcpy()/sprintf(), and with
strncpy() bounds derived from delimiters rather than the buffer size. A
malicious or MitM'd HTTP camera could overflow status_mesg[256],
connection_type[32], content_type[32] and content_boundary[64],
corrupting adjacent parser state (content_length, content_boundary_len)
for DoS and secondary heap corruption.

Add zm_strncpy(): a bounded copy that always null-terminates (strcpy
overflows; strncpy skips the terminator on truncation) and optionally
caps to a field length n for delimiter-bounded values whose source is
not null-terminated at the field end. Route every header copy in
GetResponse() through it. The content_boundary "--" prefix is written
separately and content_boundary_len is taken from strlen() of the
resulting (now bounded) string so the compare length at the multipart
subheader match stays correct.

The two subcontent-header strncpy() calls were already bounded to
sizeof-1 with an explicit terminator and are left as-is.

Adds a tests/zm_utils.cpp self-check covering fit, truncation,
delimiter-limit, delimiter-over-buffer, and empty-source cases.

Refs GHSA-93j4-rcp9-9jx6.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: connortechnology

src/zm_remote_camera_http.cpp

@@ -719,21 +719,18 @@ int RemoteCameraHttp::GetResponse() {
           start_ptr = http_header;
           end_ptr = start_ptr+strspn(start_ptr, "10.");
 
-          // FIXME Why are we memsetting every time?  Can we not do it once?
-          //memset(http_version, 0, sizeof(http_version));
-          strncpy(http_version, start_ptr, end_ptr-start_ptr);
+          zm_strncpy(http_version, start_ptr, sizeof(http_version), end_ptr-start_ptr);
 
           start_ptr = end_ptr;
           start_ptr += strspn(start_ptr, " ");
           end_ptr = start_ptr+strspn(start_ptr, "0123456789");
 
-          memset(status_code, 0, sizeof(status_code));
-          strncpy(status_code, start_ptr, end_ptr-start_ptr);
+          zm_strncpy(status_code, start_ptr, sizeof(status_code), end_ptr-start_ptr);
           int status = atoi(status_code);
 
           start_ptr = end_ptr;
           start_ptr += strspn(start_ptr, " ");
-          strcpy(status_mesg, start_ptr);
+          zm_strncpy(status_mesg, start_ptr, sizeof(status_mesg));
 
           if (status == 401) {
             if (mNeedAuth) {
@@ -771,10 +768,8 @@ int RemoteCameraHttp::GetResponse() {
           Debug(3, "Got status '%d' (%s), http version %s", status, status_mesg, http_version);
 
           if (connection_header) {
-            memset(connection_type, 0, sizeof(connection_type));
             start_ptr = connection_header + strspn(connection_header, " ");
-            // FIXME Should we not use strncpy?
-            strcpy(connection_type, start_ptr);
+            zm_strncpy(connection_type, start_ptr, sizeof(connection_type));
             Debug(3, "Got connection '%s'", connection_type);
           }
           if (content_length_header) {
@@ -786,21 +781,23 @@ int RemoteCameraHttp::GetResponse() {
             //memset(content_type, 0, sizeof(content_type));
             start_ptr = content_type_header + strspn(content_type_header, " ");
             if ( (end_ptr = strchr(start_ptr, ';')) ) {
-              strncpy(content_type, start_ptr, end_ptr-start_ptr);
+              zm_strncpy(content_type, start_ptr, sizeof(content_type), end_ptr-start_ptr);
               Debug(3, "Got content type '%s'", content_type);
 
               start_ptr = end_ptr + strspn(end_ptr, "; ");
 
               if (strncasecmp(start_ptr, boundary_match, boundary_match_len) == 0) {
                 start_ptr += boundary_match_len;
                 start_ptr += strspn(start_ptr, "-");
-                content_boundary_len = sprintf(content_boundary, "--%s", start_ptr);
+                zm_strncpy(content_boundary, "--", sizeof(content_boundary));
+                zm_strncpy(content_boundary+2, start_ptr, sizeof(content_boundary)-2);
+                content_boundary_len = strlen(content_boundary);
                 Debug(3, "Got content boundary '%s'", content_boundary);
               } else {
                 Error("No content boundary found in header '%s'", content_type_header);
               }
             } else {
-              strcpy(content_type, start_ptr);
+              zm_strncpy(content_type, start_ptr, sizeof(content_type));
               Debug(3, "Got content type '%s'", content_type);
             }
           } // end if content_type_header
@@ -940,7 +937,7 @@ int RemoteCameraHttp::GetResponse() {
           if (subcontent_type_header[0]) {
             //memset(content_type, 0, sizeof(content_type));
             start_ptr = subcontent_type_header + strspn(subcontent_type_header, " ");
-            strcpy(content_type, start_ptr);
+            zm_strncpy(content_type, start_ptr, sizeof(content_type));
             Debug(3, "Got subcontent type '%s'", content_type);
           }
           state = CONTENT;

src/zm_utils.h

@@ -32,6 +32,7 @@
 #include <string>
 #include <sys/time.h>
 #include <vector>
+#include <cstring>
 
 
 #ifdef NDEBUG
@@ -43,6 +44,19 @@
 
 typedef std::vector<std::string> StringVector;
 
+// Bounded string copy into a fixed-size buffer that ALWAYS null-terminates
+// (unlike strcpy, which overflows, and strncpy, which skips the terminator on
+// truncation). Copies at most n bytes from src when n >= 0 (for
+// delimiter-bounded fields where src is not null-terminated at the field end),
+// otherwise up to src's null terminator. dst holds `size` bytes.
+inline void zm_strncpy(char *dst, const char *src, size_t size, ssize_t n = -1) {
+  if (!size) return;
+  size_t len = (n >= 0) ? static_cast<size_t>(n) : strlen(src);
+  if (len >= size) len = size - 1;
+  memcpy(dst, src, len);
+  dst[len] = '\0';
+}
+
 std::string escape_json_string(std::string input);
 std::string remove_newlines(std::string input);
 std::string Trim(const std::string &str, const std::string &char_set);

tests/zm_utils.cpp

@@ -330,3 +330,30 @@ TEST_CASE("remove_authentication") {
     REQUIRE(result == "http://192.168.1.1");
   }
 }
+
+TEST_CASE("zm_strncpy") {
+  char buf[8];
+
+  SECTION("fits, null-terminated") {
+    zm_strncpy(buf, "abc", sizeof(buf));
+    REQUIRE(std::string(buf) == "abc");
+  }
+  SECTION("oversize truncated to size-1 and terminated") {
+    zm_strncpy(buf, "abcdefghijkl", sizeof(buf));
+    REQUIRE(std::string(buf) == "abcdefg");   // 7 chars + '\0'
+    REQUIRE(buf[7] == '\0');
+  }
+  SECTION("delimiter length n limits copy") {
+    zm_strncpy(buf, "abcdef", sizeof(buf), 3); // src not null-terminated at field end
+    REQUIRE(std::string(buf) == "abc");
+  }
+  SECTION("delimiter length also capped to buffer") {
+    zm_strncpy(buf, "abcdefghij", sizeof(buf), 20);
+    REQUIRE(std::string(buf) == "abcdefg");
+    REQUIRE(buf[7] == '\0');
+  }
+  SECTION("empty source") {
+    zm_strncpy(buf, "", sizeof(buf));
+    REQUIRE(buf[0] == '\0');
+  }
+}