summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeremy Harris <jgh146exb@wizmail.org>2017-02-08 01:19:39 +0000
committerJeremy Harris <jgh146exb@wizmail.org>2017-02-08 10:15:04 +0000
commit90341c71c19c82ba8b1cbf4d1693940b8bb8f70b (patch)
tree975408b4aa709aa62d95a9f61c3b4ee007112ba7
parentcf0812d57c63b531e2e73187508c7ae99156043c (diff)
Memory management: drop variables identified as going out-of-scope
Fixes crash in transport re-using bad $sender_ip_address from callout
-rw-r--r--doc/doc-txt/ChangeLog8
-rw-r--r--src/src/daemon.c11
-rw-r--r--src/src/exim.c41
-rw-r--r--src/src/expand.c15
-rw-r--r--src/src/queue.c33
-rw-r--r--src/src/smtp_in.c21
6 files changed, 102 insertions, 27 deletions
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 6cd472434..c23f9fe2d 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -81,6 +81,14 @@ PP/04 Bug 2018: Also handle Proxy Protocol v2 safely.
PP/05 FreeBSD compat: handle that Ports no longer create /usr/bin/perl
+JH/16 Drop variables when they go out of scope. Memory management drops a whole
+ region in one operation, for speed, and this leaves assigned pointers
+ dangling. Add checks run only under the testsuite which checks all
+ variables at a store-reset and panics on a dangling pointer; add code
+ explicitly nulling out all the variables discovered. Fixes one known
+ bug: a transport crash, where a dangling pointer for $sending_ip_address
+ originally assigned in a verify callout, is re-used.
+
Exim version 4.88
-----------------
diff --git a/src/src/daemon.c b/src/src/daemon.c
index 6e904e1f9..d4fe7759c 100644
--- a/src/src/daemon.c
+++ b/src/src/daemon.c
@@ -564,6 +564,15 @@ if (pid == 0)
/* Reclaim up the store used in accepting this message */
+ return_path = sender_address = NULL;
+ authenticated_sender = NULL;
+ sending_ip_address = NULL;
+ deliver_host_address = deliver_host =
+ deliver_domain_orig = deliver_localpart_orig = NULL;
+ dnslist_domain = dnslist_matched = NULL;
+#ifndef DISABLE_DKIM
+ dkim_cur_signer = NULL;
+#endif
store_reset(reset_point);
/* If queue_only is set or if there are too many incoming connections in
@@ -734,6 +743,8 @@ else (void)close(dup_accept_socket);
the incoming host address and an expanded active_hostname. */
log_close_all();
+interface_address =
+sender_host_address = NULL;
store_reset(reset_point);
sender_host_address = NULL;
}
diff --git a/src/src/exim.c b/src/src/exim.c
index 5e11559ad..2237476d7 100644
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -3195,7 +3195,10 @@ for (i = 1; i < argc; i++)
}
else
{
+ int old_pool = store_pool;
+ store_pool = POOL_PERM;
received_protocol = string_copyn(argrest, hn - argrest);
+ store_pool = old_pool;
sender_host_name = hn + 1;
}
}
@@ -5121,12 +5124,21 @@ if (host_checking)
if (smtp_start_session())
{
- reset_point = store_get(0);
- for (;;)
+ for (reset_point = store_get(0); ; store_reset(reset_point))
{
- store_reset(reset_point);
if (smtp_setup_msg() <= 0) break;
if (!receive_msg(FALSE)) break;
+
+ return_path = sender_address = NULL;
+ dnslist_domain = dnslist_matched = NULL;
+#ifndef DISABLE_DKIM
+ dkim_cur_signer = NULL;
+#endif
+ acl_var_m = NULL;
+ deliver_localpart_orig = NULL;
+ deliver_domain_orig = NULL;
+ callout_address = sending_ip_address = NULL;
+ sender_rate = sender_rate_limit = sender_rate_period = NULL;
}
smtp_log_no_mail();
}
@@ -5249,8 +5261,11 @@ if (smtp_input)
}
else
{
- if (received_protocol == NULL)
+ int old_pool = store_pool;
+ store_pool = POOL_PERM;
+ if (!received_protocol)
received_protocol = string_sprintf("local%s", called_as);
+ store_pool = old_pool;
set_process_info("accepting a local non-SMTP message from <%s>",
sender_address);
}
@@ -5364,7 +5379,6 @@ collapsed). */
while (more)
{
- store_reset(reset_point);
message_id[0] = 0;
/* Handle the SMTP case; call smtp_setup_mst() to deal with the initial SMTP
@@ -5406,7 +5420,7 @@ while (more)
more = receive_msg(extract_recipients);
if (message_id[0] == 0)
{
- if (more) continue;
+ if (more) goto moreloop;
smtp_log_no_mail(); /* Log no mail if configured */
exim_exit(EXIT_FAILURE);
}
@@ -5769,6 +5783,21 @@ while (more)
#ifndef SIG_IGN_WORKS
while (waitpid(-1, NULL, WNOHANG) > 0);
#endif
+
+moreloop:
+ return_path = sender_address = NULL;
+ authenticated_sender = NULL;
+ deliver_localpart_orig = NULL;
+ deliver_domain_orig = NULL;
+ deliver_host = deliver_host_address = NULL;
+ dnslist_domain = dnslist_matched = NULL;
+ malware_name = NULL;
+ callout_address = NULL;
+ sending_ip_address = NULL;
+ acl_var_m = NULL;
+ { int i; for(i=0; i<REGEX_VARS; i++) regex_vars[i] = NULL; }
+
+ store_reset(reset_point);
}
exim_exit(EXIT_SUCCESS); /* Never returns */
diff --git a/src/src/expand.c b/src/src/expand.c
index d35bf9901..d2fcd2358 100644
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -4801,8 +4801,10 @@ while (*s != 0)
port = ntohs(service_info->s_port);
}
- if ((fd = ip_connectedsocket(SOCK_STREAM, server_name, port, port,
- timeout, NULL, &expand_string_message)) < 0)
+ fd = ip_connectedsocket(SOCK_STREAM, server_name, port, port,
+ timeout, NULL, &expand_string_message);
+ callout_address = NULL;
+ if (fd < 0)
goto SOCK_FAIL;
}
@@ -7786,6 +7788,7 @@ typedef struct {
uschar * region_start;
uschar * region_end;
const uschar *var_name;
+ const uschar *var_data;
} err_ctx;
static void
@@ -7793,7 +7796,10 @@ assert_variable_notin(uschar * var_name, uschar * var_data, void * ctx)
{
err_ctx * e = ctx;
if (var_data >= e->region_start && var_data < e->region_end)
+ {
e->var_name = CUS var_name;
+ e->var_data = CUS var_data;
+ }
}
void
@@ -7821,8 +7827,9 @@ for (v = var_table; v < var_table + var_table_size; v++)
assert_variable_notin(US v->name, *(USS v->value), &e);
if (e.var_name)
- log_write(0, LOG_MAIN|LOG_PANIC_DIE, "live variable '%s' destroyed by reset_store"
- " at %s:%d\n", e.var_name, e.filename, e.linenumber);
+ log_write(0, LOG_MAIN|LOG_PANIC_DIE,
+ "live variable '%s' destroyed by reset_store at %s:%d\n- value '%.64s'",
+ e.var_name, e.filename, e.linenumber, e.var_data);
}
diff --git a/src/src/queue.c b/src/src/queue.c
index 714c73256..50e4aaef3 100644
--- a/src/src/queue.c
+++ b/src/src/queue.c
@@ -606,6 +606,9 @@ for (i = (queue_run_in_order? -1 : 0);
/* Recover store used when reading the header */
+ received_protocol = NULL;
+ sender_address = sender_ident = NULL;
+ authenticated_id = authenticated_sender = NULL;
store_reset(reset_point2);
if (!wanted) continue; /* With next message */
}
@@ -857,19 +860,16 @@ if (option >= 8) option -= 8;
/* Now scan the chain and print information, resetting store used
each time. */
-reset_point = store_get(0);
-
-for (; f != NULL; f = f->next)
+for (reset_point = store_get(0); f; f = f->next)
{
int rc, save_errno;
int size = 0;
BOOL env_read;
- store_reset(reset_point);
message_size = 0;
message_subdir[0] = f->dir_uschar;
rc = spool_read_header(f->text, FALSE, count <= 0);
- if (rc == spool_read_notopen && errno == ENOENT && count <= 0) continue;
+ if (rc == spool_read_notopen && errno == ENOENT && count <= 0) goto next;
save_errno = errno;
env_read = (rc == spool_read_OK || rc == spool_read_hdrerror);
@@ -901,8 +901,7 @@ for (; f != NULL; f = f->next)
/* Collect delivered addresses from any J file */
fname[ptr] = 'J';
- jread = Ufopen(fname, "rb");
- if (jread != NULL)
+ if ((jread = Ufopen(fname, "rb")))
{
while (Ufgets(big_buffer, big_buffer_size, jread) != NULL)
{
@@ -917,7 +916,7 @@ for (; f != NULL; f = f->next)
fprintf(stdout, "%s ", string_format_size(size, big_buffer));
for (i = 0; i < 16; i++) fputc(f->text[i], stdout);
- if (env_read && sender_address != NULL)
+ if (env_read && sender_address)
{
printf(" <%s>", sender_address);
if (sender_set_untrusted) printf(" (%s)", originator_login);
@@ -940,7 +939,7 @@ for (; f != NULL; f = f->next)
if (rc != spool_read_hdrerror)
{
printf("\n\n");
- continue;
+ goto next;
}
}
@@ -948,7 +947,7 @@ for (; f != NULL; f = f->next)
printf("\n");
- if (recipients_list != NULL)
+ if (recipients_list)
{
for (i = 0; i < recipients_count; i++)
{
@@ -957,12 +956,22 @@ for (; f != NULL; f = f->next)
if (!delivered || option != 1)
printf(" %s %s\n", (delivered != NULL)? "D":" ",
recipients_list[i].address);
- if (delivered != NULL) delivered->data.val = TRUE;
+ if (delivered) delivered->data.val = TRUE;
}
- if (option == 2 && tree_nonrecipients != NULL)
+ if (option == 2 && tree_nonrecipients)
queue_list_extras(tree_nonrecipients);
printf("\n");
}
+
+next:
+ received_protocol = NULL;
+ sender_fullhost = sender_helo_name =
+ sender_rcvhost = sender_host_address = sender_address = sender_ident = NULL;
+ sender_host_authenticated = authenticated_sender = authenticated_id = NULL;
+ interface_address = NULL;
+ acl_var_m = NULL;
+
+ store_reset(reset_point);
}
}
diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c
index 2ea5b271d..bd83de045 100644
--- a/src/src/smtp_in.c
+++ b/src/src/smtp_in.c
@@ -1878,7 +1878,6 @@ Returns: nothing
static void
smtp_reset(void *reset_point)
{
-store_reset(reset_point);
recipients_list = NULL;
rcpt_count = rcpt_defer_count = rcpt_fail_count =
raw_recipients_count = recipients_count = recipients_list_max = 0;
@@ -1901,7 +1900,12 @@ submission_mode = FALSE; /* Can be set by ACL */
suppress_local_fixups = suppress_local_fixups_default; /* Can be set by ACL */
active_local_from_check = local_from_check; /* Can be set by ACL */
active_local_sender_retain = local_sender_retain; /* Can be set by ACL */
-sender_address = NULL;
+sending_ip_address = NULL;
+return_path = sender_address = NULL;
+sender_data = NULL; /* Can be set by ACL */
+deliver_localpart_orig = NULL;
+deliver_domain_orig = NULL;
+callout_address = NULL;
submission_name = NULL; /* Can be set by ACL */
raw_sender = NULL; /* After SMTP rewrite, before qualifying */
sender_address_unrewritten = NULL; /* Set only after verify rewrite */
@@ -1914,6 +1918,7 @@ authenticated_sender = NULL;
bmi_run = 0;
bmi_verdicts = NULL;
#endif
+dnslist_domain = dnslist_matched = NULL;
#ifndef DISABLE_DKIM
dkim_signers = NULL;
dkim_disable_verify = FALSE;
@@ -1921,6 +1926,7 @@ dkim_collect_input = FALSE;
#endif
dsn_ret = 0;
dsn_envid = NULL;
+deliver_host = deliver_host_address = NULL; /* Can be set by ACL */
#ifndef DISABLE_PRDR
prdr_requested = FALSE;
#endif
@@ -1947,13 +1953,13 @@ acl_var_m = NULL;
not the first message in an SMTP session and the previous message caused them
to be referenced in an ACL. */
-if (message_body != NULL)
+if (message_body)
{
store_free(message_body);
message_body = NULL;
}
-if (message_body_end != NULL)
+if (message_body_end)
{
store_free(message_body_end);
message_body_end = NULL;
@@ -1963,12 +1969,13 @@ if (message_body_end != NULL)
repetition in the same message, but it seems right to repeat them for different
messages. */
-while (acl_warn_logged != NULL)
+while (acl_warn_logged)
{
string_item *this = acl_warn_logged;
acl_warn_logged = acl_warn_logged->next;
store_free(this);
}
+store_reset(reset_point);
}
@@ -4469,9 +4476,13 @@ while (done <= 0)
case ENV_MAIL_OPT_UTF8:
if (smtputf8_advertised)
{
+ int old_pool = store_pool;
+
DEBUG(D_receive) debug_printf("smtputf8 requested\n");
message_smtputf8 = allow_utf8_domains = TRUE;
+ store_pool = POOL_PERM;
received_protocol = string_sprintf("utf8%s", received_protocol);
+ store_pool = old_pool;
}
break;
#endif