From 2b1bccc229974a121326e20f05729d71c939cfeb Mon Sep 17 00:00:00 2001
From: rswindell <>
Date: Sun, 1 Sep 2019 09:08:40 +0000
Subject: [PATCH] Received-telnet command improvements: - If a telnet command
 was received in multiple packets, the memcpy optimization   in
 telnet_interpret() would skip/drop all bytes in the subsequent pkt payload  
 before an IAC char. Don't optimize when in the middle of a telnet command. -
 If a received telnet command exceeds the telnet_cmd buffer, reset the  
 received telnet_cmdlen and log a warning-level message - If a telnet
 sub-negotiation END command is received as the beginning of a   new telnet
 command, log a warning-level message and reset the telnet_cmdlen.

This fixes the occasional problem observed when using fTelnet and its sending
the "SEND-LOCATION" sub-neg command split between 2 TCP packets. Only
part of the first packet would be used as the location data and the rest
processed as input from the users (e.g. as the Login: prompt). Thanks, Ree!
---
 src/sbbs3/main.cpp | 44 +++++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/src/sbbs3/main.cpp b/src/sbbs3/main.cpp
index f8a2619e93..1ffcb7c0c4 100644
--- a/src/sbbs3/main.cpp
+++ b/src/sbbs3/main.cpp
@@ -1469,8 +1469,9 @@ static BYTE* telnet_interpret(sbbs_t* sbbs, BYTE* inbuf, int inlen,
 	BYTE*	first_cr=NULL;
 	int 	i;
 
+	outlen=0;
+
 	if(inlen<1) {
-		outlen=0;
 		return(inbuf);	// no length? No interpretation
 	}
 
@@ -1485,19 +1486,20 @@ static BYTE* telnet_interpret(sbbs_t* sbbs, BYTE* inbuf, int inlen,
 			first_cr=(BYTE*)memchr(inbuf, CR, inlen);
 	}
 
-    if(!sbbs->telnet_cmdlen	&& first_iac==NULL && first_cr==NULL) {
-        outlen=inlen;
-        return(inbuf);	// no interpretation needed
-    }
+    if(!sbbs->telnet_cmdlen) {
+		if(first_iac==NULL && first_cr==NULL) {
+			outlen=inlen;
+			return(inbuf);	// no interpretation needed
+		}
 
-    if(first_iac!=NULL || first_cr!=NULL) {
-		if(first_iac!=NULL && (first_cr==NULL || first_iac<first_cr))
-   			outlen=first_iac-inbuf;
-		else
-			outlen=first_cr-inbuf;
-	    memcpy(outbuf, inbuf, outlen);
-    } else
-    	outlen=0;
+		if(first_iac!=NULL || first_cr!=NULL) {
+			if(first_iac!=NULL && (first_cr==NULL || first_iac<first_cr))
+   				outlen=first_iac-inbuf;
+			else
+				outlen=first_cr-inbuf;
+			memcpy(outbuf, inbuf, outlen);
+		}
+	}
 
     for(i=outlen;i<inlen;i++) {
 		if(!(sbbs->telnet_mode&TELNET_MODE_GATE)
@@ -1524,11 +1526,21 @@ static BYTE* telnet_interpret(sbbs_t* sbbs, BYTE* inbuf, int inlen,
 
 			if(sbbs->telnet_cmdlen<sizeof(sbbs->telnet_cmd))
 				sbbs->telnet_cmd[sbbs->telnet_cmdlen++]=inbuf[i];
+			else {
+				lprintf(LOG_WARNING, "Node %d telnet command (%d, %d) buffer limit reached (%u bytes)"
+					,sbbs->cfg.node_num, sbbs->telnet_cmd[1], sbbs->telnet_cmd[2], sbbs->telnet_cmdlen);
+				sbbs->telnet_cmdlen = 0;
+			}
 
 			uchar command	= sbbs->telnet_cmd[1];
 			uchar option	= sbbs->telnet_cmd[2];
 
-			if(sbbs->telnet_cmdlen>=2 && command==TELNET_SB) {
+			if(sbbs->telnet_cmdlen == 2 && command == TELNET_SE) {
+				lprintf(LOG_WARNING, "Node %d unexpected telnet sub-negotiation END command"
+					,sbbs->cfg.node_num);
+				sbbs->telnet_cmdlen = 0;
+			}
+			else if(sbbs->telnet_cmdlen>=2 && command==TELNET_SB) {
 				if(inbuf[i]==TELNET_SE
 					&& sbbs->telnet_cmd[sbbs->telnet_cmdlen-2]==TELNET_IAC) {
 
@@ -1633,7 +1645,7 @@ static BYTE* telnet_interpret(sbbs_t* sbbs, BYTE* inbuf, int inlen,
 							,sbbs->telnet_cmd[3]);
 					sbbs->telnet_cmdlen=0;
 				}
-			}
+			} // Sub-negotiation command
             else if(sbbs->telnet_cmdlen==2 && inbuf[i]<TELNET_WILL) {
 				if(startup->options&BBS_OPT_DEBUG_TELNET)
             		lprintf(LOG_DEBUG,"Node %d %s telnet cmd: %s"
@@ -1725,9 +1737,7 @@ static BYTE* telnet_interpret(sbbs_t* sbbs, BYTE* inbuf, int inlen,
 #endif
 					}
 				}
-
                 sbbs->telnet_cmdlen=0;
-
             }
 			if(sbbs->telnet_mode&TELNET_MODE_GATE)	// Pass-through commands
 				outbuf[outlen++]=inbuf[i];
-- 
GitLab