From 9b1a34b3fa24979e61935f40cd8ef06c72f914e3 Mon Sep 17 00:00:00 2001
From: "Rob Swindell (on Windows 11)" <rob@synchro.net>
Date: Sun, 14 Jan 2024 19:26:46 -0800
Subject: [PATCH] Don't expand @-codes automatically for all node
 messages/telegrams saved

This recent enhancement (Commit b27bd426) introduced security and usability
concerns.

So I created (and am now using where requested) a wrapper for formatting
text.dat/ini strings which will automaticlaly detect @-code encoded strings
and expand/use them *only* (instead of printf %-specifiers).

This might impact issue #696 since although unintentionally, it actually was
possible to mix @-codes and %-specifier usage in certain (node status)
text.dat/ini strings, but that should not be possible now. It's either/or:
@-codes or %-specifiers, not both.
---
 src/sbbs3/logon.cpp    | 10 ++--------
 src/sbbs3/logout.cpp   | 10 ++--------
 src/sbbs3/postmsg.cpp  | 17 ++++++-----------
 src/sbbs3/putmsg.cpp   |  6 ++----
 src/sbbs3/readmail.cpp | 15 +++++----------
 src/sbbs3/readmsgs.cpp |  9 ++-------
 src/sbbs3/sbbs.h       |  3 +++
 src/sbbs3/str.cpp      | 24 ++++++++++++++++++++++++
 src/sbbs3/text.h       |  2 +-
 src/sbbs3/textgen.c    |  2 +-
 10 files changed, 48 insertions(+), 50 deletions(-)

diff --git a/src/sbbs3/logon.cpp b/src/sbbs3/logon.cpp
index c6f7c18407..bf0db2a79e 100644
--- a/src/sbbs3/logon.cpp
+++ b/src/sbbs3/logon.cpp
@@ -554,16 +554,10 @@ bool sbbs_t::logon()
 			if(thisnode.status!=NODE_QUIET
 				&& (node.status==NODE_INUSE || node.status==NODE_QUIET)
 				&& !(node.misc&NODE_AOFF) && node.useron!=useron.number) {
-				char fmt[256];
-				expand_atcodes(text[NodeLoggedOnAtNbps], fmt, sizeof fmt);
-				if(strcmp(text[NodeLoggedOnAtNbps], fmt) != 0)
-					SAFECOPY(str, fmt);
-				else
-					safe_snprintf(str, sizeof str, fmt
+				putnmsg(i, format_text(NodeLoggedOnAtNbps
 						,cfg.node_num
 						,thisnode.misc&NODE_ANON ? text[UNKNOWN_USER] : useron.alias
-						,connection);
-				putnmsg(i, str);
+						,connection));
 			} 
 		}
 
diff --git a/src/sbbs3/logout.cpp b/src/sbbs3/logout.cpp
index d7cf3c56f7..6910051a3b 100644
--- a/src/sbbs3/logout.cpp
+++ b/src/sbbs3/logout.cpp
@@ -66,16 +66,10 @@ void sbbs_t::logout()
 				getnodedat(i,&node,0);
 				if((node.status==NODE_INUSE || node.status==NODE_QUIET)
 					&& !(node.misc&NODE_AOFF) && node.useron!=useron.number) {
-					char fmt[256];
-					expand_atcodes(text[NodeLoggedOff], fmt, sizeof fmt);
-					if(strcmp(text[NodeLoggedOff], fmt) != 0)
-						SAFECOPY(str, fmt);
-					else
-						snprintf(str, sizeof str, fmt
+					putnmsg(i, format_text(NodeLoggedOff
 							,cfg.node_num
 							,thisnode.misc&NODE_ANON
-								? text[UNKNOWN_USER] : useron.alias);
-					putnmsg(i, str);
+								? text[UNKNOWN_USER] : useron.alias));
 				}
 			}
 
diff --git a/src/sbbs3/postmsg.cpp b/src/sbbs3/postmsg.cpp
index a683cb10f9..df331dbe1d 100644
--- a/src/sbbs3/postmsg.cpp
+++ b/src/sbbs3/postmsg.cpp
@@ -67,7 +67,6 @@ static uchar* findsig(char* msgbuf)
 bool sbbs_t::postmsg(int subnum, int wm_mode, smb_t* resmb, smbmsg_t* remsg)
 {
 	char	str[256];
-	char	fmt[256];
 	char	title[LEN_TITLE+1] = "";
 	char	top[256] = "";
 	char	touser[64] = "";
@@ -108,16 +107,12 @@ bool sbbs_t::postmsg(int subnum, int wm_mode, smb_t* resmb, smbmsg_t* remsg)
 		if(remsg->to != NULL)
 			strListPush(&names, remsg->to);
 		msgattr=(ushort)(remsg->hdr.attr&MSG_PRIVATE);
-		expand_atcodes(text[RegardingByToOn], fmt, sizeof fmt, remsg);
-		if(strcmp(text[RegardingByToOn], fmt) != 0)
-			SAFECOPY(top, fmt);
-		else
-			snprintf(top, sizeof top, fmt
-				,title
-				,from
-				,msghdr_field(remsg, remsg->to, NULL, term_supports(UTF8))
-				,timestr(remsg->hdr.when_written.time)
-				,smb_zonestr(remsg->hdr.when_written.zone,NULL));
+		SAFECOPY(top, format_text(RegardingByToOn, remsg
+			,title
+			,from
+			,msghdr_field(remsg, remsg->to, NULL, term_supports(UTF8))
+			,timestr(remsg->hdr.when_written.time)
+			,smb_zonestr(remsg->hdr.when_written.zone,NULL)));
 		if(remsg->tags != NULL)
 			SAFECOPY(tags, remsg->tags);
 	}
diff --git a/src/sbbs3/putmsg.cpp b/src/sbbs3/putmsg.cpp
index 8fd446289c..7e23625607 100644
--- a/src/sbbs3/putmsg.cpp
+++ b/src/sbbs3/putmsg.cpp
@@ -556,13 +556,11 @@ char sbbs_t::putmsgfrag(const char* buf, int& mode, int org_cols, JSObject* obj)
 // Write short message (telegram) to user, supports @-codes
 bool sbbs_t::putsmsg(int user_num, const char* str)
 {
-	char buf[256];
-	return ::putsmsg(&cfg, user_num, expand_atcodes(str, buf, sizeof buf)) == 0;
+	return ::putsmsg(&cfg, user_num, (char*)str) == 0;
 }
 
 // Write short message to node in-use, supports @-codes
 bool sbbs_t::putnmsg(int node_num, const char* str)
 {
-	char buf[256];
-	return ::putnmsg(&cfg, node_num, expand_atcodes(str, buf, sizeof buf)) == 0;
+	return ::putnmsg(&cfg, node_num, (char*)str) == 0;
 }
diff --git a/src/sbbs3/readmail.cpp b/src/sbbs3/readmail.cpp
index b1109d2c65..2fdfb336ce 100644
--- a/src/sbbs3/readmail.cpp
+++ b/src/sbbs3/readmail.cpp
@@ -43,7 +43,6 @@ int sbbs_t::readmail(uint usernumber, int which, int lm_mode)
 {
 	char	str[256],str2[256],done=0,domsg=1
 			,*p;
-	char	fmt[256];
 	char 	tmp[512];
 	int		i;
 	uint32_t u,v;
@@ -344,17 +343,13 @@ int sbbs_t::readmail(uint usernumber, int which, int lm_mode)
 				msg.hdr.number=msg.idx.number;
 				smb_getmsgidx(&smb,&msg);
 
-				const char* text_str;
+				enum text text_num;
 				if(!stricmp(str2,str))		/* Reply to sender */
-					text_str = text[Regarding];
+					text_num = Regarding;
 				else						/* Reply to other */
-					text_str = text[RegardingByOn];
-				expand_atcodes(text_str, fmt, sizeof fmt, &msg);
-				if(strcmp(text_str, fmt) != 0)
-					SAFECOPY(str2, fmt);
-				else
-					SAFEPRINTF3(str2, fmt, msghdr_field(&msg, msg.subj), msghdr_field(&msg, msg.from, tmp)
-						,timestr(msg.hdr.when_written.time));
+					text_num = RegardingByOn;
+				SAFECOPY(str2, format_text(text_num, msghdr_field(&msg, msg.subj), msghdr_field(&msg, msg.from, tmp)
+						,timestr(msg.hdr.when_written.time)));
 
 				p=strrchr(str,'@');
 				if(p) { 							/* name @addr */
diff --git a/src/sbbs3/readmsgs.cpp b/src/sbbs3/readmsgs.cpp
index c451745de9..c8a8c90b97 100644
--- a/src/sbbs3/readmsgs.cpp
+++ b/src/sbbs3/readmsgs.cpp
@@ -399,7 +399,6 @@ int sbbs_t::scanposts(int subnum, int mode, const char *find)
 {
 	char	str[256],str2[256],do_find=true,mismatches=0
 			,done=0,domsg=1,*buf;
-	char	fmt[256];
 	char	find_buf[128];
 	char	tmp[128];
 	int		i;
@@ -1010,13 +1009,9 @@ int sbbs_t::scanposts(int subnum, int mode, const char *find)
 					&& stricmp(msg.to,useron.name)
 					&& stricmp(msg.to,useron.alias))
 					break;
-				expand_atcodes(text[Regarding], fmt, sizeof fmt, &msg);
-				if(strcmp(text[Regarding], fmt) != 0)
-					SAFECOPY(str2, fmt);
-				else
-					SAFEPRINTF2(str2, fmt
+				SAFECOPY(str2, format_text(Regarding
 						,msghdr_field(&msg, msg.subj)
-						,timestr(msg.hdr.when_written.time));
+						,timestr(msg.hdr.when_written.time)));
 				if(msg.from_net.addr==NULL)
 					SAFECOPY(str,msg.from);
 				else if(msg.from_net.type==NET_FIDO)
diff --git a/src/sbbs3/sbbs.h b/src/sbbs3/sbbs.h
index 612da35e58..58462a8f84 100644
--- a/src/sbbs3/sbbs.h
+++ b/src/sbbs3/sbbs.h
@@ -742,6 +742,9 @@ public:
 	bool	gettimeleft_inside = false;
 
 	/* str.cpp */
+	char	format_text_buf[256]{};
+	char*	format_text(enum text, ...);
+	char*	format_text(enum text, smbmsg_t*, ...);
 	int		get_text_num(const char* id);
 	const char* get_text(const char* id);
 	bool	load_user_text(void);
diff --git a/src/sbbs3/str.cpp b/src/sbbs3/str.cpp
index 35079972bb..87dad92a28 100644
--- a/src/sbbs3/str.cpp
+++ b/src/sbbs3/str.cpp
@@ -123,6 +123,30 @@ bool sbbs_t::load_user_text(void)
 	return replace_text(path);
 }
 
+char* sbbs_t::format_text(enum text num, ...)
+{
+	expand_atcodes(text[num], format_text_buf, sizeof format_text_buf, /* remsg: */NULL);
+	if(strcmp(text[num], format_text_buf) == 0) { // No @-codes expanded
+		va_list args;
+		va_start(args, num);
+		vsnprintf(format_text_buf, sizeof format_text_buf, text[num], args);
+		va_end(args);
+	}
+	return format_text_buf;
+}
+
+char* sbbs_t::format_text(enum text num, smbmsg_t* remsg, ...)
+{
+	expand_atcodes(text[num], format_text_buf, sizeof format_text_buf, remsg);
+	if(strcmp(text[num], format_text_buf) == 0) { // No @-codes expanded
+		va_list args;
+		va_start(args, remsg);
+		vsnprintf(format_text_buf, sizeof format_text_buf, text[num], args);
+		va_end(args);
+	}
+	return format_text_buf;
+}
+
 /****************************************************************************/
 /* Lists all users who have access to the current sub.                      */
 /****************************************************************************/
diff --git a/src/sbbs3/text.h b/src/sbbs3/text.h
index 51ef87bd98..333e0891b2 100644
--- a/src/sbbs3/text.h
+++ b/src/sbbs3/text.h
@@ -16,7 +16,7 @@ extern
 #endif
 const char* const text_id[];
 
-enum {
+enum text {
 	 MsgSubj
 	,MsgAttr
 	,MsgTo
diff --git a/src/sbbs3/textgen.c b/src/sbbs3/textgen.c
index 4e0a7e9503..ce59408766 100644
--- a/src/sbbs3/textgen.c
+++ b/src/sbbs3/textgen.c
@@ -198,7 +198,7 @@ int main(int argc, char **argv)
 	fputs("\"C\"\n", text_h);
 	fputs("#endif\n", text_h);
 	fputs("const char* const text_id[];\n\n", text_h);
-	fputs("enum {\n",text_h);
+	fputs("enum text {\n",text_h);
 
 	if(argc > 2)
 		p = argv[2];
-- 
GitLab