Fix writing to tls socket

[michaelortmann]

Found by: https://github.com/michaelortmann/
Patch by: https://github.com/michaelortmann/
Fixes:

One-line summary:
Fix writing to tls socket

Additional description (if needed):
SSL_write() in tputs()

eggdrop/src/net.c

Line 1346 in 890e7e4

x = SSL_write(socklist[i].ssl, s, len);

can trigger callback function ssl_info()
which can call debug1():

eggdrop/src/tls.c

Lines 900 to 902 in 890e7e4

	if (err & (SSL_ERROR_WANT_READ | SSL_ERROR_WANT_WRITE)) {
	/* Errors to be ignored for non-blocking */
	debug1("TLS: awaiting more %s", (err & SSL_ERROR_WANT_READ) ? "reads" : "writes");

which can destroy memory by a chain of events like dprintf() -> dprint() -> out_dcc_general() -> escape_telnet() with the following static buffer:

eggdrop/src/dcc.c

Line 95 in 890e7e4

static char buf[1024];

In this case, during SSL_write(), garbage (len bytes of the debug message) will be written to the tls socket.

This PR changes the escape_telnet() function, so it doesnt use a static buffer anymore. Only pros, no cons.

This bug is since at least eggdrop 1.8.3rc1, #497

Test cases demonstrating functionality (if applicable):
> openssl s_client -connect 127.0.0.1:3343
Before:

read R BLOCK
[11:BotA  (Eggdrop v1.10.1+iprehash (C) 1997 Robey Pointer (C) 1999-2025 Eggheads Development Team)

Please enter your handle.

After:

read R BLOCK


BotA  (Eggdrop v1.10.1+iprehash (C) 1997 Robey Pointer (C) 1999-2025 Eggheads Development Team)

Please enter your handle.

[michaelortmann]

There are 2 ways to fix this. This is the 2nd way:

diff --git a/src/net.c b/src/net.c
index 1fd934ce..66b4614b 100644
--- a/src/net.c
+++ b/src/net.c
@@ -1343,7 +1343,15 @@ void tputs(int z, char *s, unsigned int len)
       }
 #ifdef TLS
       if (socklist[i].ssl) {
+
+        /* SSL_write can call ssl_info() which can call tputs() and destroy s,
+        * so lets make a copy
+        */
+        char s2[LOGLINELEN];
+        memcpy(s2, s, len);
         x = SSL_write(socklist[i].ssl, s, len);
+        memcpy(s, s2, len);
+
         if (x < 0) {
           int err = SSL_get_error(socklist[i].ssl, x);
           if (err == SSL_ERROR_WANT_WRITE || err == SSL_ERROR_WANT_READ)

Please review and tell me, which way you prefer.
The 2nd way is better because we dont remove a debug message, like we do with the 1st patch
But the 2nd way is worse, because it does 2 memcpys to achieve this.

[michaelortmann]

if you dont know also, then pick the 1st way of fix, thats the PR in its current state. What i want to avoid is this PR getting stuck because of no decision on that matter.

[michaelortmann]

I hunted some more.
The re-entrance issue is not really tputs(), but a chain of events like dprintf() -> dprint() -> out_dcc_general() -> escape_telnet() with the following static buffer:

eggdrop/src/dcc.c

Line 95 in 890e7e4

static char buf[1024];

[vanosg]

Can you resolve conflicts when you get a chance? Thanks!

[michaelortmann]

Can you resolve conflicts when you get a chance? Thanks!

done.