From b25b5734921f5aacf95ecc67d636664f1a83b329 Mon Sep 17 00:00:00 2001
From: "Rob Swindell (on Windows)" <rob@synchro.net>
Date: Thu, 13 Apr 2023 14:41:32 -0700
Subject: [PATCH] Fix false "SUSPECTED BOUNCE ATTACK ATTEMPT" for IPv6 FTP-data
 connections

This bug only impacted non-passive FTP connections. Using an FTP client
with active (not passive) data connections over an IPv6 connection would
false-trigger the "bounce attack" detection and the FTP server responded with
"504 Bad port number" and logged a hack attempt in data/hack.log.

The issue was that we were comparing the socket address structure (which
contains other fields besides the address itself) between the control and
proposed-data connections. While this logic worked okay for IPv4,
it did not for IPv6 (the 2 structs contained some non-address differences).
Rather than modify the socket address structures to match where needed, I'm
just comparing the string representation of the addresses, since that's
what we really care about anyway.

Thank to "mark i" of Truck Stop BBS for alerting me to this issue
---
 src/sbbs3/ftpsrvr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/sbbs3/ftpsrvr.c b/src/sbbs3/ftpsrvr.c
index a453d86661..097a6b028d 100644
--- a/src/sbbs3/ftpsrvr.c
+++ b/src/sbbs3/ftpsrvr.c
@@ -2905,7 +2905,7 @@ static void ctrl_thread(void* arg)
 			inet_addrtop(&data_addr, data_ip, sizeof(data_ip));
 			bool bounce_allowed = (startup->options & FTP_OPT_ALLOW_BOUNCE) && !(user.rest & FLAG('G'));
 			if(data_port < IPPORT_RESERVED
-				|| (memcmp(&data_addr, &ftp.client_addr, ftp.client_addr_len) != 0 && !bounce_allowed)) {
+				|| (strcmp(data_ip, host_ip) != 0 && !bounce_allowed)) {
 				lprintf(LOG_WARNING,"%04d <%s> !SUSPECTED BOUNCE ATTACK ATTEMPT to %s port %u"
 					,sock,user.alias
 					,data_ip,data_port);
-- 
GitLab