Try to fix memory issues.

7 files changed, 90 insertions, 71 deletions

diff --git a/src/filter.c b/src/filter.c
index d41c85ff0..68a43d1f9 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -84,35 +84,37 @@ insert_metric_result (struct worker_task *task, struct metric *metric, const gch
 	/* Add metric score */
 
 
-	if ((s = g_hash_table_lookup (metric_res->symbols, symbol)) != NULL) {
-		if (!single) {
-			if (s->options && opts && opts != s->options) {
-				/* Append new options */
-				s->options = g_list_concat (s->options, g_list_copy(opts));
-				/*
-				 * Note that there is no need to add new destructor of GList as elements of appended
-				 * GList are used directly, so just free initial GList
-				 */
-			}
-			else if (opts) {
-				s->options = opts;
-				memory_pool_add_destructor (task->task_pool, (pool_destruct_func) g_list_free, s->options);
-			}
-
-			s->score += w;
-			metric_res->score += w;
+	if (!single && (s = g_hash_table_lookup (metric_res->symbols, symbol)) != NULL) {
+		if (s->options && opts && opts != s->options) {
+			/* Append new options */
+			s->options = g_list_concat (s->options, g_list_copy(opts));
+			/*
+			 * Note that there is no need to add new destructor of GList as elements of appended
+			 * GList are used directly, so just free initial GList
+			 */
 		}
+		else if (opts) {
+			s->options = g_list_copy (opts);
+			memory_pool_add_destructor (task->task_pool, (pool_destruct_func) g_list_free, s->options);
+		}
+
+		s->score += w;
+		metric_res->score += w;
 	}
 	else {
 		s = memory_pool_alloc (task->task_pool, sizeof (struct symbol));
 		s->score = w;
-		s->options = opts;
+
 		s->name = symbol;
 		metric_res->score += w;
 
 		if (opts) {
+			s->options = g_list_copy (opts);
 			memory_pool_add_destructor (task->task_pool, (pool_destruct_func) g_list_free, s->options);
 		}
+		else {
+			s->options = NULL;
+		}
 
 		g_hash_table_insert (metric_res->symbols, (gpointer) symbol, s);
 	}
@@ -165,6 +167,11 @@ insert_result_common (struct worker_task *task, const gchar *symbol, double flag
 			cur = g_list_next (cur);
 		}
 	}
+
+	if (opts != NULL) {
+		/* XXX: it is not wise to destroy them here */
+		g_list_free (opts);
+	}
 }
 
 /* Insert result that may be increased on next insertions */
diff --git a/src/html.c b/src/html.c
index 772f7d33b..2ab441ad3 100644
--- a/src/html.c
+++ b/src/html.c
@@ -679,14 +679,22 @@ html_strncasestr (const gchar *s, const gchar *find, gsize len)
 }
 
 static void
-check_phishing (struct worker_task *task, struct uri *href_url, const gchar *url_text)
+check_phishing (struct worker_task *task, struct uri *href_url, const gchar *url_text, gsize remain)
 {
 	struct uri                     *new;
 	gchar                          *url_str;
-	gsize                           len;
+	const gchar                    *p;
+	gsize                           len = 0;
 	gint                            off, rc;
 
-	len = strcspn (url_text, "<>");
+	p = url_text;
+	while (len < remain) {
+		if (*p == '<' || *p == '>') {
+			break;
+		}
+		len ++;
+		p ++;
+	}
 
 	if (url_try_text (task->task_pool, url_text, len, &off, &url_str) && url_str != NULL) {
 		new = memory_pool_alloc0 (task->task_pool, sizeof (struct uri));
@@ -796,7 +804,7 @@ parse_tag_url (struct worker_task *task, struct mime_text_part *part, tag_id_t i
 			 * Check for phishing
 			 */
 			if ((p = strchr (c, '>')) != NULL ) {
-				check_phishing (task, url, p + 1);
+				check_phishing (task, url, p + 1, tag_len - (p - tag_text));
 			}
 			if (part->html_urls && g_tree_lookup (part->html_urls, url_text) == NULL) {
 				g_tree_insert (part->html_urls, url_text, url);
diff --git a/src/message.c b/src/message.c
index 0a88ab0f7..86e333dc1 100644
--- a/src/message.c
+++ b/src/message.c
@@ -837,7 +837,9 @@ mime_foreach_callback (GMimeObject * part, gpointer user_data)
 			msg_err ("endless recursion detected: %d", task->parser_recursion);
 			return;
 		}
+#ifndef GMIME24
 		g_object_unref (message);
+#endif
 	}
 	else if (GMIME_IS_MESSAGE_PARTIAL (part)) {
 		/* message/partial */
diff --git a/src/plugins/regexp.c b/src/plugins/regexp.c
index 8afefb9da..0ab4451e1 100644
--- a/src/plugins/regexp.c
+++ b/src/plugins/regexp.c
@@ -607,7 +607,7 @@ process_regexp (struct rspamd_regexp *re, struct worker_task *task, const gchar
 	guint8                         *ct;
 	gsize                           clen;
 	gint                            r, passed = 0, start, end, old;
-	gboolean                        matched;
+	gboolean                        matched = FALSE;
 
 	GList                          *cur, *headerlist;
 	GRegex                         *regexp;
diff --git a/src/plugins/spf.c b/src/plugins/spf.c
index f7b0caab3..7b7dac605 100644
--- a/src/plugins/spf.c
+++ b/src/plugins/spf.c
@@ -177,11 +177,6 @@ spf_plugin_callback (struct spf_record *record, struct worker_task *task)
             }
 			cur = g_list_previous (cur);
 		}
-		if (record->addrs != NULL) {
-			/* Free addresses that we already proceed */
-			g_list_free (record->addrs);
-			record->addrs = NULL;
-		}
 	}
 
 	if (task->save.saved == 0) {
diff --git a/src/spf.c b/src/spf.c
index 212bd0074..f19665e79 100644
--- a/src/spf.c
+++ b/src/spf.c
@@ -145,6 +145,19 @@ dump_spf_record (GList *addrs)
 	msg_info ("spf record: %s", logbuf);
 }
 
+/*
+ * Destructor for spf record
+ */
+static void
+spf_record_destructor (gpointer r)
+{
+	struct spf_record              *rec = r;
+
+	if (rec->addrs) {
+		g_list_free (rec->addrs);
+	}
+}
+
 static gboolean
 parse_spf_ipmask (const gchar *begin, struct spf_addr *addr)
 {
@@ -292,6 +305,7 @@ spf_record_dns_callback (struct rspamd_dns_reply *reply, gpointer arg)
 					else if (reply->type == DNS_REQUEST_A) {
 						if (cb->addr->addr == 0) {
 							cb->addr->addr = ntohl (elt_data->a.addr[0].s_addr);
+							cb->addr->mask = 32;
 						}
 						else {
 							/* Insert one more address */
@@ -313,6 +327,7 @@ spf_record_dns_callback (struct rspamd_dns_reply *reply, gpointer arg)
 					if (reply->type == DNS_REQUEST_A) {
 						/* XXX: process only one record */
 						cb->addr->addr = ntohl (elt_data->a.addr[0].s_addr);
+						cb->addr->mask = 32;
 					}
 					break;
 				case SPF_RESOLVE_PTR:
@@ -344,50 +359,32 @@ spf_record_dns_callback (struct rspamd_dns_reply *reply, gpointer arg)
 						start_spf_parse (cb->rec, begin);
 						cb->rec->in_include = FALSE;
 
-						if (tmp) {
+						if (tmp && cb->rec->addrs) {
 							tmp1 = g_list_find (tmp, cb->addr);
 							if (tmp1) {
 								/* Insert new list in place of include element */
+								cb->rec->addrs->prev = tmp1->prev;
 								last = g_list_last (cb->rec->addrs);
-
-								if (tmp1->prev == NULL && tmp1->next == NULL) {
-									g_list_free1 (tmp1);
+								last->next = tmp1;
+								tmp1->prev = last;
+								/* If tmp1 is at the beginning of the list, shift list */
+								if (tmp == tmp1) {
+									tmp = cb->rec->addrs;
 								}
 								else {
-
-									if (tmp1->prev) {
-										tmp1->prev->next = cb->rec->addrs;
-									}
-									else {
-										/* Elt is the first element, so we need to shift temporary list */
-										tmp = tmp1->next;
-										tmp->prev = NULL;
-									}
-									if (tmp1->next) {
-										tmp1->next->prev = last;
-										if (last != NULL) {
-											last->next = tmp1->next;
-										}
-									}
-
-									if (cb->rec->addrs != NULL) {
-										cb->rec->addrs->prev = tmp1->prev;
-									}
-
-									/* Shift temporary list */
-									while (tmp->prev) {
-										tmp = tmp->prev;
-									}
-
-									cb->rec->addrs = tmp;
-									g_list_free1 (tmp1);
+									tmp1->prev->next = cb->rec->addrs;
+								}
+								/* Remove self element */
+								tmp = g_list_delete_link (tmp, tmp1);
 #ifdef SPF_DEBUG
-									msg_info ("after include");
-									dump_spf_record (cb->rec->addrs);
+								msg_info ("after include");
+								dump_spf_record (cb->rec->addrs);
 #endif
-								}
 							}
 						}
+						if (tmp) {
+							cb->rec->addrs = tmp;
+						}
 					}
 					break;
 				case SPF_RESOLVE_EXP:
@@ -410,16 +407,19 @@ spf_record_dns_callback (struct rspamd_dns_reply *reply, gpointer arg)
 					if (reply->type == DNS_REQUEST_MX) {
 						msg_info ("cannot find MX record for %s", cb->rec->cur_domain);
 						cb->addr->addr = ntohl (INADDR_NONE);
+						cb->addr->mask = 32;
 					}
 					else if (reply->type != DNS_REQUEST_MX) {
 						msg_info ("cannot resolve MX record for %s", cb->rec->cur_domain);
 						cb->addr->addr = ntohl (INADDR_NONE);
+						cb->addr->mask = 32;
 					}
 					break;
 				case SPF_RESOLVE_A:
 					if (reply->type == DNS_REQUEST_A) {
 						/* XXX: process only one record */
 						cb->addr->addr = ntohl (INADDR_NONE);
+						cb->addr->mask = 32;
 					}
 					break;
 				case SPF_RESOLVE_PTR:
@@ -434,6 +434,7 @@ spf_record_dns_callback (struct rspamd_dns_reply *reply, gpointer arg)
 					break;
 				case SPF_RESOLVE_EXISTS:
 					cb->addr->addr = ntohl (INADDR_NONE);
+					cb->addr->mask = 32;
 					break;
 		}
 	}
@@ -909,6 +910,8 @@ expand_spf_macro (struct worker_task *task, struct spf_record *rec, gchar *begin
 	(x) = memory_pool_alloc (task->task_pool, sizeof (struct spf_addr));		\
 	(x)->mech = check_spf_mech (rec->cur_elt, &need_shift);						\
 	(x)->spf_string = memory_pool_strdup (task->task_pool, begin);				\
+	(x)->addr = 0;																\
+	(x)->mask = 32;																\
 } while (0);
 
 /* Read current element and try to parse record */
@@ -1149,7 +1152,10 @@ resolve_spf (struct worker_task *task, spf_cb_t callback)
 	rec = memory_pool_alloc0 (task->task_pool, sizeof (struct spf_record));
 	rec->task = task;
 	rec->callback = callback;
+	/* Add destructor */
+	memory_pool_add_destructor (task->task_pool, (pool_destruct_func)spf_record_destructor, rec);
 
+	/* Extract from data */
 	if (task->from && (domain = strchr (task->from, '@')) != NULL && *domain == '@') {
 		rec->sender = task->from;
 
@@ -1170,6 +1176,7 @@ resolve_spf (struct worker_task *task, spf_cb_t callback)
 		}
 	}
 	else {
+		/* Extract from header */
 		domains = message_get_header (task->task_pool, task->message, "From", FALSE);
 
 		if (domains != NULL) {
diff --git a/src/url.c b/src/url.c
index 8282f9217..83eb30d4d 100644
--- a/src/url.c
+++ b/src/url.c
@@ -930,7 +930,7 @@ url_file_end (const gchar *begin, const gchar *end, const gchar *pos, url_match_
 		}
 	}
 
-	while (p < end && *p != stop && is_urlsafe (*p)) {
+	while (p < end - 1 && *p != stop && is_urlsafe (*p)) {
 		p ++;
 	}
 
@@ -979,14 +979,14 @@ url_web_end (const gchar *begin, const gchar *end, const gchar *pos, url_match_t
 	if (is_atom (*p)) {
 		/* might be a domain or user@domain */
 		c = p;
-		while (p < end) {
+		while (p < end - 1) {
 			if (!is_atom (*p)) {
 				break;
 			}
 
 			p++;
 
-			while (p < end && is_atom (*p)) {
+			while (p < end - 1 && is_atom (*p)) {
 				p++;
 			}
 
@@ -1006,18 +1006,18 @@ url_web_end (const gchar *begin, const gchar *end, const gchar *pos, url_match_t
 	}
 	else if (is_domain (*p)) {
 domain:
-		while (p < end) {
+		while (p < end - 1) {
 			if (!is_domain (*p)) {
 				break;
 			}
 
 			p++;
 
-			while (p < end && is_domain (*p)) {
+			while (p < end - 1 && is_domain (*p)) {
 				p++;
 			}
 
-			if ((p + 1) < end && *p == '.' && (is_domain (*(p + 1)) || *(p + 1) == '/')) {
+			if ((p + 1) < end - 1 && *p == '.' && (is_domain (*(p + 1)) || *(p + 1) == '/')) {
 				p++;
 			}
 		}
@@ -1034,7 +1034,7 @@ domain:
 			if (is_digit (*p) || passwd) {
 				port = (*p++ - '0');
 
-				while (p < end && is_digit (*p) && port < 65536) {
+				while (p < end - 1 && is_digit (*p) && port < 65536) {
 					port = (port * 10) + (*p++ - '0');
 				}
 
@@ -1052,7 +1052,7 @@ domain:
 				passwd = TRUE;
 				c = p;
 
-				while (p < end && is_atom (*p)) {
+				while (p < end - 1 && is_atom (*p)) {
 					p++;
 				}
 
@@ -1076,7 +1076,7 @@ domain:
 		case '/': /* we've detected a path component to our url */
 			p++;
 		case '?':
-			while (p < end && is_urlsafe (*p)) {
+			while (p < end - 1 && is_urlsafe (*p)) {
 				if (*p == open_brace) {
 					brace_stack++;
 				}