Introduce some helpful macros for terminating strings, and fix overrun in dcgettext.

5 files changed, 58 insertions, 36 deletions

diff --git a/ChangeLog b/ChangeLog
index 2df4e170..44eb563c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,19 @@
 2017-01-27         Andrew J. Schorr     <aschorr@telemetry-investments.com>
 
+	* awk.h (str_terminate_f): New helper function for terminating a string
+	NODE.
+	(str_terminate): Macro wrapper to call str_terminate_f.
+	(str_restore): New macro to restore the string.
+	* builtin.c (do_strftime): Use str_terminate and str_restore.
+	(do_dcgettext): Ditto, and remove saved_end flag since equivalent
+	to testing (t2 != NULL). Fix overrun bug in calculating result
+	length when !ENABLE_NLS.
+	(do_dcngettext, do_bindtextdomain): Use str_terminate and str_restore.
+	* interpret.h (Op_arrayfor_init, Op_indirect_func_call): Ditto.
+	* str_array.c (env_remove): Ditto.
+
+2017-01-27         Andrew J. Schorr     <aschorr@telemetry-investments.com>
+
 	* interpret.h [UNFIELD]: Fix condition for assignment from
 	value with valref == 1. Fixes problems introduced at gawk 4.1.2.
 
diff --git a/awk.h b/awk.h
index 9a5c94a8..4ad9c717 100644
--- a/awk.h
+++ b/awk.h
@@ -1958,3 +1958,19 @@ erealloc_real(void *ptr, size_t count, const char *where, const char *var, const
 
 	return ret;
 }
+
+/*
+ * str_terminate_f, str_terminate, str_restore: function and macros to
+ * reduce chances of typos when terminating and restoring strings.
+ * This also helps to enforce that the NODE must be in scope when we restore.
+ */
+
+static inline void
+str_terminate_f(NODE *n, char *savep)
+{
+	*savep = n->stptr[n->stlen];
+	n->stptr[n->stlen] = '\0';
+}
+
+#define str_terminate(n, save) str_terminate_f((n), &save)
+#define str_restore(n, save) (n)->stptr[(n)->stlen] = save
diff --git a/builtin.c b/builtin.c
index faee54ee..14f5c730 100644
--- a/builtin.c
+++ b/builtin.c
@@ -1981,8 +1981,7 @@ do_strftime(int nargs)
 			DEREF(t1);
 			return make_string("", 0);
 		}
-		save = format[formatlen];
-		t1->stptr[formatlen] = '\0';
+		str_terminate(t1, save);
 	}
 
 	if (do_gmt)
@@ -2021,7 +2020,7 @@ do_strftime(int nargs)
 		efree(bufp);
 done:
 	if (t1) {
-		t1->stptr[formatlen] = save;
+		str_restore(t1, save);
 		DEREF(t1);
 	}
 	return ret;
@@ -3751,8 +3750,8 @@ do_dcgettext(int nargs)
 #if ENABLE_NLS && defined(LC_MESSAGES) && HAVE_DCGETTEXT
 	int lc_cat;
 	char *domain;
-	char save, save1;
-	bool saved_end = false;
+	char save1, save2;
+	size_t reslen;
 
 	if (nargs == 3) {	/* third argument */
 		tmp = POP_STRING();
@@ -3764,9 +3763,7 @@ do_dcgettext(int nargs)
 	if (nargs >= 2) {  /* second argument */
 		t2 = POP_STRING();
 		domain = t2->stptr;
-		save = domain[t2->stlen];
-		domain[t2->stlen] = '\0';
-		saved_end = true;
+		str_terminate(t2, save2);
 	} else
 		domain = TEXTDOMAIN;
 #else
@@ -3782,21 +3779,22 @@ do_dcgettext(int nargs)
 
 	t1 = POP_STRING();	/* first argument */
 	string = t1->stptr;
-	save1 = string[t1->stlen];
-	string[t1->stlen] = '\0';
 
 #if ENABLE_NLS && defined(LC_MESSAGES) && HAVE_DCGETTEXT
+	str_terminate(t1, save1);
 	the_result = dcgettext(domain, string, lc_cat);
-	string[t1->stlen] = save1;
-	if (saved_end)
-		domain[t2->stlen] = save;
-	if (t2 != NULL)
+	str_restore(t1, save1);
+	if (t2 != NULL) {
+		str_restore(t2, save2);
 		DEREF(t2);
+	}
+	reslen = strlen(the_result);
 #else
 	the_result = string;
+	reslen = t1->stlen;
 #endif
 	DEREF(t1);
-	return make_string(the_result, strlen(the_result));
+	return make_string(the_result, reslen);
 }
 
 
@@ -3855,14 +3853,12 @@ do_dcngettext(int nargs)
 
 #if ENABLE_NLS && defined(LC_MESSAGES) && HAVE_DCGETTEXT
 
-	save1 = string1[t1->stlen];
-	string1[t1->stlen] = '\0';
-	save2 = string2[t2->stlen];
-	string2[t2->stlen] = '\0';
+	str_terminate(t1, save1);
+	str_terminate(t2, save2);
 	the_result = dcngettext(domain, string1, string2, number, lc_cat);
 	reslen = strlen(the_result);
-	string1[t1->stlen] = save1;
-	string2[t2->stlen] = save2;
+	str_restore(t1, save1);
+	str_restore(t2, save2);
 	if (saved_end)
 		domain[t3->stlen] = save;
 	if (t3 != NULL)
@@ -3917,13 +3913,12 @@ do_bindtextdomain(int nargs)
 	t1 = POP_STRING();
 	if (t1->stlen > 0) {
 		directory = (const char *) t1->stptr;
-		save1 = t1->stptr[t1->stlen];
-		t1->stptr[t1->stlen] = '\0';
+		str_terminate(t1, save1);
 	}
 
 	the_result = bindtextdomain(domain, directory);
 	if (directory)
-		t1->stptr[t1->stlen] = save1;
+		str_restore(t1, save1);
 
 	DEREF(t1);
 	if (t2 != NULL) {
diff --git a/interpret.h b/interpret.h
index 191e1efb..13394e22 100644
--- a/interpret.h
+++ b/interpret.h
@@ -912,15 +912,14 @@ mod:
 				sort_str = force_string(sort_str);
 				if (sort_str->stlen > 0) {
 					how_to_sort = sort_str->stptr;
-					save = sort_str->stptr[sort_str->stlen];
-					sort_str->stptr[sort_str->stlen] = '\0';
+					str_terminate(sort_str, save);
 					saved_end = true;
 				}
 			}
 
 			list = assoc_list(array, how_to_sort, SORTED_IN);
 			if (saved_end)
-				sort_str->stptr[sort_str->stlen] = save;
+				str_restore(sort_str, save);
 
 arrayfor:
 			getnode(r);
@@ -1066,15 +1065,14 @@ match_re:
 				fatal(_("indirect function call requires a simple scalar value"));
 
 			t1 = force_string(t1);
-			save = t1->stptr[t1->stlen];
-			t1->stptr[t1->stlen] = '\0';
+			str_terminate(t1, save);
 			if (t1->stlen > 0) {
 				/* retrieve function definition node */
 				f = pc->func_body;
 				if (f != NULL && strcmp(f->vname, t1->stptr) == 0) {
 					/* indirect var hasn't been reassigned */
 
-					t1->stptr[t1->stlen] = save;
+					str_restore(t1, save);
 					ni = setup_frame(pc);
 					JUMPTO(ni);	/* Op_func */
 				}
@@ -1099,12 +1097,12 @@ match_re:
 					r = call_split_func(t1->stptr, arg_count);
 				else
 					r = the_func(arg_count);
-				t1->stptr[t1->stlen] = save;
+				str_restore(t1, save);
 
 				PUSH(r);
 				break;
 			} else if (f->type != Node_func) {
-				t1->stptr[t1->stlen] = save;
+				str_restore(t1, save);
 				if (f->type == Node_ext_func) {
 					/* code copied from below, keep in sync */
 					INSTRUCTION *bc;
@@ -1129,7 +1127,7 @@ match_re:
 							pc->func_name);
 			}
 			pc->func_body = f;     /* save for next call */
-			t1->stptr[t1->stlen] = save;
+			str_restore(t1, save);
 
 			ni = setup_frame(pc);
 			JUMPTO(ni);	/* Op_func */
diff --git a/str_array.c b/str_array.c
index c559a39a..fe07ce4b 100644
--- a/str_array.c
+++ b/str_array.c
@@ -776,10 +776,9 @@ env_remove(NODE *symbol, NODE *subs)
 	char save;
 
 	if (val != NULL) {
-		save = subs->stptr[subs->stlen];
-		subs->stptr[subs->stlen] = '\0';
+		str_terminate(subs, save);
 		(void) unsetenv(subs->stptr);
-		subs->stptr[subs->stlen] = save;
+		str_restore(subs, save);
 	}
 
 	return val;