Issue proftpd#903: Ensure that we do not reuse already-destroyed memo…

…ry pools

during data transfers.

src/data.c

@@ -2,7 +2,7 @@
  * ProFTPD - FTP server daemon
  * Copyright (c) 1997, 1998 Public Flood Software
  * Copyright (c) 1999, 2000 MacGyver aka Habeeb J. Dihu <macgyver@tos.net>
- * Copyright (c) 2001-2018 The ProFTPD Project team
+ * Copyright (c) 2001-2020 The ProFTPD Project team
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -684,7 +684,7 @@ void pr_data_close(int quiet) {
  */
 void pr_data_cleanup(void) {
   /* sanity check */
-  if (session.d) {
+  if (session.d != NULL) {
     pr_inet_lingering_close(session.pool, session.d, timeout_linger);
     session.d = NULL;
   }
@@ -711,7 +711,7 @@ void pr_data_abort(int err, int quiet) {
     strerror(err), err, quiet ? "true" : "false",
     true_abort ? "true" : "false");
 
-  if (session.d) {
+  if (session.d != NULL) {
     if (true_abort == FALSE) {
       pr_inet_lingering_close(session.pool, session.d, timeout_linger);
 
@@ -893,6 +893,11 @@ void pr_data_abort(int err, int quiet) {
     if (true_abort == FALSE) {
       pr_response_add_err(respcode, _("Transfer aborted. %s"), msg ? msg : "");
     }
+
+    /* Forcibly clear the data-transfer instigating command pool from the
+     * Response API.
+     */
+    pr_response_set_pool(NULL);
   }
 
   if (true_abort) {
@@ -925,6 +930,7 @@ static void poll_ctrl(void) {
     res = pr_cmd_read(&cmd);
     if (res < 0) {
       int xerrno;
+
 #if defined(ECONNABORTED)
       xerrno = ECONNABORTED;
 #elif defined(ENOTCONN)
@@ -993,8 +999,8 @@ static void poll_ctrl(void) {
 
         pr_response_flush(&resp_err_list);
 
-        destroy_pool(cmd->pool);
         pr_response_set_pool(resp_pool);
+        destroy_pool(cmd->pool);
 
       /* We don't want to actually dispatch the NOOP command, since that
        * would overwrite the scoreboard with the NOOP state; admins probably
@@ -1019,13 +1025,14 @@ static void poll_ctrl(void) {
 
         pr_response_flush(&resp_list);
 
-        destroy_pool(cmd->pool);
         pr_response_set_pool(resp_pool);
+        destroy_pool(cmd->pool);
 
       } else {
         char *title_buf = NULL;
-        int title_len = -1;
-        const char *sce_cmd = NULL, *sce_cmd_arg = NULL;
+        int curr_cmd_id = 0, title_len = -1;
+        const char *curr_cmd = NULL, *sce_cmd = NULL, *sce_cmd_arg = NULL;
+        cmd_rec *curr_cmd_rec = NULL;
 
         pr_trace_msg(trace_channel, 5,
           "client sent '%s' command during data transfer, dispatching",
@@ -1037,6 +1044,9 @@ static void poll_ctrl(void) {
           pr_proctitle_get(title_buf, title_len + 1); 
         }
 
+        curr_cmd = session.curr_cmd;
+        curr_cmd_id = session.curr_cmd_id;
+        curr_cmd_rec = session.curr_cmd_rec;
         sce_cmd = pr_scoreboard_entry_get(PR_SCORE_CMD);
         sce_cmd_arg = pr_scoreboard_entry_get(PR_SCORE_CMD_ARG);
 
@@ -1052,6 +1062,9 @@ static void poll_ctrl(void) {
         }
 
         destroy_pool(cmd->pool);
+        session.curr_cmd = curr_cmd;
+        session.curr_cmd_id = curr_cmd_id;
+        session.curr_cmd_rec = curr_cmd_rec;
       }
 
     } else {

src/main.c

@@ -900,8 +900,7 @@ static void cmd_loop(server_rec *server, conn_t *c) {
       pr_timer_reset(PR_TIMER_IDLE, ANY_MODULE);
     }
 
-    if (cmd) {
-
+    if (cmd != NULL) {
       /* Detect known commands for other protocols; if found, drop the
        * connection, lest we be used as part of an attack on a different
        * protocol server (Bug#4143).
@@ -917,6 +916,9 @@ static void cmd_loop(server_rec *server, conn_t *c) {
 
       pr_cmd_dispatch(cmd);
       destroy_pool(cmd->pool);
+      session.curr_cmd = NULL;
+      session.curr_cmd_id = 0;
+      session.curr_cmd_rec = NULL;
 
     } else {
       pr_event_generate("core.invalid-command", NULL);

src/response.c

@@ -219,6 +219,12 @@ void pr_response_add_err(const char *numeric, const char *fmt, ...) {
     return;
   }
 
+  if (resp_pool == NULL) {
+    pr_trace_msg(trace_channel, 1,
+      "no response pool set, ignoring added %s error response", numeric);
+    return;
+  }
+
   va_start(msg, fmt);
   res = vsnprintf(resp_buf, sizeof(resp_buf), fmt, msg);
   va_end(msg);
@@ -272,6 +278,12 @@ void pr_response_add(const char *numeric, const char *fmt, ...) {
     return;
   }
 
+  if (resp_pool == NULL) {
+    pr_trace_msg(trace_channel, 1,
+      "no response pool set, ignoring added %s response", numeric);
+    return;
+  }
+
   va_start(msg, fmt);
   res = vsnprintf(resp_buf, sizeof(resp_buf), fmt, msg);
   va_end(msg);

tests/api/data.c

@@ -1,6 +1,6 @@
 /*
  * ProFTPD - FTP server testsuite
- * Copyright (c) 2015-2018 The ProFTPD Project team
+ * Copyright (c) 2015-2020 The ProFTPD Project team
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by

tests/api/response.c

@@ -87,6 +87,11 @@ START_TEST (response_add_test) {
   const char *last_resp_code = NULL, *last_resp_msg = NULL;
   char *resp_code = R_200, *resp_msg = "OK";
 
+  pr_response_set_pool(NULL);
+
+  mark_point();
+  pr_response_add(resp_code, "%s", resp_msg);
+
   pr_response_set_pool(p);
 
   mark_point();
@@ -118,6 +123,11 @@ START_TEST (response_add_err_test) {
   const char *last_resp_code = NULL, *last_resp_msg = NULL;
   char *resp_code = R_450, *resp_msg = "Busy";
 
+  pr_response_set_pool(NULL);
+
+  mark_point();
+  pr_response_add(resp_code, "%s", resp_msg);
+
   pr_response_set_pool(p);
 
   mark_point();