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() https://github.com/eggheads/eggdrop/blob/890e7e4ddc82eeb9b52c747002b24e7dad69687f/src/net.c#L1346 can trigger callback function ssl_info() which can call debug1(): https://github.com/eggheads/eggdrop/blob/890e7e4ddc82eeb9b52c747002b24e7dad69687f/src/tls.c#L900-L902 which can destroy memory by a chain of events like dprintf() -> dprint() -> out_dcc_general() -> escape_telnet() with the following static buffer: https://github.com/eggheads/eggdrop/blob/890e7e4ddc82eeb9b52c747002b24e7dad69687f/src/dcc.c#L95 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: https://github.com/eggheads/eggdrop/blob/890e7e4ddc82eeb9b52c747002b24e7dad69687f/src/dcc.c#L95