From 4212494106b7115eafd5d612dd91f2966ec3b71f Mon Sep 17 00:00:00 2001
From: Simon Arlott <sa.me.uk>
Date: Sat, 5 Aug 2017 11:27:02 +0100
Subject: [PATCH] ircd: substitution: fix buffer overrun if variable name is
 too long

Also fix the "ptr = ptr + (pptr - ptr)" aka "ptr = pptr" mess by
removing pptr.
---
 ircd/substitution.c   | 40 +++++++++++++---------------------------
 tests/substitution1.c | 20 ++++++++++++++++++++
 2 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/ircd/substitution.c b/ircd/substitution.c
index 63d5f2f2..24672c7b 100644
--- a/ircd/substitution.c
+++ b/ircd/substitution.c
@@ -101,45 +101,30 @@ char *substitution_parse(const char *fmt, rb_dlink_list *varlist)
 	const char *ptr;
 	char *bptr = buf;
 
-	for (ptr = fmt; *ptr != '\0' && bptr - buf < BUFSIZE; ptr++)
-		if (*ptr != '$')
+	for (ptr = fmt; *ptr != '\0' && bptr - buf < BUFSIZE; ptr++) {
+		if (*ptr != '$') {
 			*bptr++ = *ptr;
-		else if (*(ptr + 1) == '{')
-		{
-			static char varname[BUFSIZE];
+		} else if (*(ptr + 1) == '{') {
+			char varname[BUFSIZE] = { 0 };
 			char *vptr = varname;
-			const char *pptr;
 			rb_dlink_node *nptr;
 
-			*vptr = '\0';
-
 			/* break out ${var} */
-			for (pptr = ptr + 2; *pptr != '\0'; pptr++)
-			{
-				if (*pptr == '$')
-				{
-					*vptr++ = '\0';
-					pptr--;
+			for (ptr += 2; *ptr != '\0'; ptr++) {
+				if (*ptr == '$') {
+					ptr--;
 					break;
-				}
-				else if (*pptr != '}')
-					*vptr++ = *pptr;
-				else
-				{
-					*vptr++ = '\0';
+				} else if (*ptr == '}') {
 					break;
+				} else if (vptr < &varname[sizeof(varname) - 1]) {
+					*vptr++ = *ptr;
 				}
 			}
 
-			/* advance ptr by length of variable */
-			ptr += (pptr - ptr);
-
-			RB_DLINK_FOREACH(nptr, varlist->head)
-			{
+			RB_DLINK_FOREACH(nptr, varlist->head) {
 				struct substitution_variable *val = (struct substitution_variable *) nptr->data;
 
-				if (!rb_strcasecmp(varname, val->name))
-				{
+				if (!rb_strcasecmp(varname, val->name)) {
 					rb_strlcpy(bptr, val->value, sizeof(buf) - (bptr - buf));
 					bptr += strlen(val->value);
 					if (bptr >= &buf[sizeof(buf)]) {
@@ -153,6 +138,7 @@ char *substitution_parse(const char *fmt, rb_dlink_list *varlist)
 			if (*ptr == '\0')
 				break;
 		}
+	}
 
 	*bptr = '\0';
 	return buf;
diff --git a/tests/substitution1.c b/tests/substitution1.c
index 0dfb897d..343819e7 100644
--- a/tests/substitution1.c
+++ b/tests/substitution1.c
@@ -270,6 +270,16 @@ static void too_long_variable3c(void) {
 	is_string(tmp2, substitution_parse("${text200}${text400}${text100}", &varlist2), MSG);
 }
 
+static void long_variable_name1(void) {
+	char input[2048];
+
+	strcpy(input, "${");
+	strcat(input, MKTEXT(1000));
+	strcat(input, "}");
+
+	is_string("test", substitution_parse(input, &varlist2), MSG);
+}
+
 int main(int argc, char *argv[])
 {
 	memset(&me, 0, sizeof(me));
@@ -303,6 +313,11 @@ int main(int argc, char *argv[])
 	substitution_append_var(&varlist2, "text514", MKTEXT(514));
 	substitution_append_var(&varlist2, "text600", MKTEXT(600));
 
+	char temp[2048];
+	strcpy(temp, MKTEXT(1000));
+	temp[BUFSIZE - 1] = '\0';
+	substitution_append_var(&varlist2, temp, "test");
+
 	plan_lazy();
 
 	valid_variable1();
@@ -336,5 +351,10 @@ int main(int argc, char *argv[])
 	too_long_variable3b();
 	too_long_variable3c();
 
+	long_variable_name1();
+
+	substitution_free(&varlist);
+	substitution_free(&varlist2);
+
 	return 0;
 }