From 0739d2a68c1e3fa52e836eea0b951336982f4b97 Mon Sep 17 00:00:00 2001
From: Ben Meadors <benmmeadors@gmail.com>
Date: Wed, 12 Aug 2026 19:12:32 -0500
Subject: [PATCH] fix(raspihttp): memory safety and error-path leaks in
 PiWebServer (#11451)

- handleAPIv1ToRadio: the toRadio handler memcpy'd a fixed 512 bytes
  out of the request body regardless of its actual size. ulfius
  allocates binary_body at exactly binary_body_length bytes and leaves
  it NULL for a body-less PUT, so short bodies caused a heap overread
  and empty ones dereferenced NULL. The unclamped length (ulfius
  accepts up to 1024) was also handed to handleToRadio, reading past
  the 512-byte stack buffer. Clamp both directions, matching the ESP32
  ContentHandler behavior.

- callback_static_file: the stream-free callback only runs when
  ulfius_set_stream_response succeeds, so the failure branch leaked the
  FILE (and its fd) on every failed request. Close it and return 500.

- CheckSSLandLoad: free cert_pem before the missing-key return; the
  constructor retries after regenerating certs and the reload
  overwrote (leaked) the first buffer.

- generate_rsa_key / CreateSSLCertificate: free the EVP_PKEY_CTX,
  EVP_PKEY, and X509 on their error returns; previously only the
  success paths released them.
---
 src/mesh/raspihttp/PiWebServer.cpp | 34 +++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/mesh/raspihttp/PiWebServer.cpp b/src/mesh/raspihttp/PiWebServer.cpp
index 9158a3061..5dae68a73 100644
--- a/src/mesh/raspihttp/PiWebServer.cpp
+++ b/src/mesh/raspihttp/PiWebServer.cpp
@@ -206,6 +206,10 @@ int callback_static_file(const struct _u_request *request, struct _u_response *r
                     if (ulfius_set_stream_response(response, 200, callback_static_file_stream, callback_static_file_stream_free,
                                                    length, STATIC_FILE_CHUNK, f) != U_OK) {
                         LOG_DEBUG("callback_static_file - Error ulfius_set_stream_response");
+                        // The stream-free callback only runs when the stream was accepted, so the
+                        // file must be closed here or the FILE and its fd leak on every failure.
+                        fclose(f);
+                        ulfius_set_string_body_response(response, 500, "Internal server error");
                     }
                 }
             } else {
@@ -256,9 +260,13 @@ int handleAPIv1ToRadio(const struct _u_request *req, struct _u_response *res, vo
     }
 
     byte buffer[MAX_TO_FROM_RADIO_SIZE];
-    size_t s = req->binary_body_length;
-
-    memcpy(buffer, req->binary_body, MAX_TO_FROM_RADIO_SIZE);
+    // ulfius allocates binary_body at exactly binary_body_length bytes (NULL for a body-less
+    // PUT), and the framework accepts bodies larger than our buffer, so clamp both directions.
+    size_t s = req->binary_body ? req->binary_body_length : 0;
+    if (s > sizeof(buffer))
+        s = sizeof(buffer);
+    if (s > 0)
+        memcpy(buffer, req->binary_body, s);
 
     // FIXME* Problem with portdunio loosing mountpoint maybe because of running in a real sep. thread
 
@@ -327,12 +335,11 @@ int generate_rsa_key(EVP_PKEY **pkey)
     EVP_PKEY_CTX *pkey_ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_RSA, NULL);
     if (!pkey_ctx)
         return -1;
-    if (EVP_PKEY_keygen_init(pkey_ctx) <= 0)
-        return -1;
-    if (EVP_PKEY_CTX_set_rsa_keygen_bits(pkey_ctx, 2048) <= 0)
-        return -1;
-    if (EVP_PKEY_keygen(pkey_ctx, pkey) <= 0)
+    if (EVP_PKEY_keygen_init(pkey_ctx) <= 0 || EVP_PKEY_CTX_set_rsa_keygen_bits(pkey_ctx, 2048) <= 0 ||
+        EVP_PKEY_keygen(pkey_ctx, pkey) <= 0) {
+        EVP_PKEY_CTX_free(pkey_ctx);
         return -1;
+    }
     EVP_PKEY_CTX_free(pkey_ctx);
     return 0; // SUCCESS
 }
@@ -413,6 +420,10 @@ int PiWebServerThread::CheckSSLandLoad()
     key_pem = read_file_into_string(KEY_PATH);
     if (key_pem == NULL) {
         LOG_ERROR("File private_key can't be loaded or missing");
+        // The constructor retries CheckSSLandLoad() after regenerating, which would overwrite
+        // (and leak) the cert buffer loaded above.
+        free(cert_pem);
+        cert_pem = NULL;
         return 2;
     }
 
@@ -432,6 +443,9 @@ int PiWebServerThread::CreateSSLCertificate()
 
     if (generate_self_signed_x509(pkey, &x509) != 0) {
         LOG_ERROR("Error generating X509-Cert");
+        // generate_self_signed_x509 can fail after allocating *x509; X509_free(NULL) is a no-op
+        X509_free(x509);
+        EVP_PKEY_free(pkey);
         return 2;
     }
 
@@ -439,6 +453,8 @@ int PiWebServerThread::CreateSSLCertificate()
     FILE *pkey_file = fopen(KEY_PATH, "wb");
     if (!pkey_file) {
         LOG_ERROR("Error opening private key file");
+        X509_free(x509);
+        EVP_PKEY_free(pkey);
         return 3;
     }
     // write private key file
@@ -449,6 +465,8 @@ int PiWebServerThread::CreateSSLCertificate()
     FILE *x509_file = fopen(CERT_PATH, "wb");
     if (!x509_file) {
         LOG_ERROR("Error opening cert");
+        X509_free(x509);
+        EVP_PKEY_free(pkey);
         return 4;
     }
     // write certificate