From: Christopher Faulet <cfaulet@haproxy.com>
Date: Tue, 30 Jun 2020 16:52:32 +0000 (+0200)
Subject: BUG/MEDIUM: pattern: Add a trailing \0 to match strings only if possible
X-Git-Tag: v2.2-dev12~41
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b4cf7ab9bc413bbb956e225f903959bff17e4049;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: pattern: Add a trailing \0 to match strings only if possible

In pat_match_str() and pat_math_beg() functions, a trailing zero is
systematically added at the end of the string, even if the buffer is not large
enough to accommodate it. It is a possible buffer overflow. For instance, when
the alpn is matched against a list of strings, the sample fetch is filled with a
non-null terminated string returned by the SSL library. No trailing zero must be
added at the end of this string, because it is outside the buffer.

So, to fix the bug, a trailing zero is added only if the buffer is large enough
to accommodate it. Otherwise, the sample fetch is duplicated. smp_dup() function
adds a trailing zero to the duplicated string, truncating it if it is too long.

This patch should fix the issue #718. It must be backported to all supported
versions.
---

diff --git a/src/pattern.c b/src/pattern.c
index 766badec1c..416e809638 100644
--- a/src/pattern.c
+++ b/src/pattern.c
@@ -453,7 +453,6 @@ struct pattern *pat_match_str(struct sample *smp, struct pattern_expr *expr, int
 {
 	int icase;
 	struct ebmb_node *node;
-	char prev;
 	struct pattern_tree *elt;
 	struct pattern_list *lst;
 	struct pattern *pattern;
@@ -462,14 +461,27 @@ struct pattern *pat_match_str(struct sample *smp, struct pattern_expr *expr, int
 
 	/* Lookup a string in the expression's pattern tree. */
 	if (!eb_is_empty(&expr->pattern_tree)) {
-		/* we may have to force a trailing zero on the test pattern */
-		prev = smp->data.u.str.area[smp->data.u.str.data];
-		if (prev)
-			smp->data.u.str.area[smp->data.u.str.data] = '\0';
+		char prev = 0;
+
+		if (smp->data.u.str.data < smp->data.u.str.size) {
+			/* we may have to force a trailing zero on the test pattern and
+			 * the buffer is large enough to accommodate it.
+			 */
+			prev = smp->data.u.str.area[smp->data.u.str.data];
+			if (prev)
+				smp->data.u.str.area[smp->data.u.str.data] = '\0';
+		}
+		else {
+			/* Otherwise, the sample is duplicated. A trailing zero
+			 * is automatically added to the string.
+			 */
+			if (!smp_dup(smp))
+				return NULL;
+		}
+
 		node = ebst_lookup(&expr->pattern_tree, smp->data.u.str.area);
 		if (prev)
 			smp->data.u.str.area[smp->data.u.str.data] = prev;
-
 		if (node) {
 			if (fill) {
 				elt = ebmb_entry(node, struct pattern_tree, node);
@@ -618,7 +630,6 @@ struct pattern *pat_match_beg(struct sample *smp, struct pattern_expr *expr, int
 {
 	int icase;
 	struct ebmb_node *node;
-	char prev;
 	struct pattern_tree *elt;
 	struct pattern_list *lst;
 	struct pattern *pattern;
@@ -627,10 +638,24 @@ struct pattern *pat_match_beg(struct sample *smp, struct pattern_expr *expr, int
 
 	/* Lookup a string in the expression's pattern tree. */
 	if (!eb_is_empty(&expr->pattern_tree)) {
-		/* we may have to force a trailing zero on the test pattern */
-		prev = smp->data.u.str.area[smp->data.u.str.data];
-		if (prev)
-			smp->data.u.str.area[smp->data.u.str.data] = '\0';
+		char prev = 0;
+
+		if (smp->data.u.str.data < smp->data.u.str.size) {
+			/* we may have to force a trailing zero on the test pattern and
+			 * the buffer is large enough to accommodate it.
+			 */
+			prev = smp->data.u.str.area[smp->data.u.str.data];
+			if (prev)
+				smp->data.u.str.area[smp->data.u.str.data] = '\0';
+		}
+		else {
+			/* Otherwise, the sample is duplicated. A trailing zero
+			 * is automatically added to the string.
+			 */
+			if (!smp_dup(smp))
+				return NULL;
+		}
+
 		node = ebmb_lookup_longest(&expr->pattern_tree,
 					   smp->data.u.str.area);
 		if (prev)