[Openvpn-devel,v1] socks: In establish_socks_proxy_udpassoc check result of recv_socks_reply

Message ID 20260114135807.20637-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v1] socks: In establish_socks_proxy_udpassoc check result of recv_socks_reply | expand

Commit Message

Gert Doering Jan. 14, 2026, 1:58 p.m. UTC
From: Frank Lichtenheld <frank@lichtenheld.com>

Not just check the return value but also that relay_addr
is valid. recv_socks_reply doesn't care whether the answer
is what we expected. This is probably a very unlikely edge
case but it doesn't hurt to check for it here.

Reported-By: Joshua Rogers <contact@joshua.hu>
Found-By: ZeroPath (https://zeropath.com)
Github: openvpn-private-issues#13
Change-Id: Ic1c8f24de423541bdc85e70b5a688213800d86de
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1469
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1469
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Jan. 14, 2026, 6:40 p.m. UTC | #1
We don't currently have "known failing" SOCKS test in our t_client setup
(maybe we should, but the release t_client.sh script has no concept of
"this test is expected to fail" yet), so this is not automatically tested.

That said, the change makes sense, and will hopefully ease someone's
future debugging when talking to a SOCKS proxy that is refusing
cooperation.

Your patch has been applied to the master branch.

commit b2cc5c2ec49c3cfc428f0ecef9684ac318845a84
Author: Frank Lichtenheld
Date:   Wed Jan 14 14:58:02 2026 +0100

     socks: In establish_socks_proxy_udpassoc check result of recv_socks_reply

     Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1469
     Message-Id: <20260114135807.20637-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg35249.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/socks.c b/src/openvpn/socks.c
index 078b4e1..5cb5912 100644
--- a/src/openvpn/socks.c
+++ b/src/openvpn/socks.c
@@ -412,8 +412,7 @@ 
             send(ctrl_sd, "\x05\x03\x00\x01\x00\x00\x00\x00\x00\x00", 10, MSG_NOSIGNAL);
         if (size != 10)
         {
-            msg(D_LINK_ERRORS | M_ERRNO,
-                "establish_socks_proxy_passthru: TCP port write failed on send()");
+            msg(D_LINK_ERRORS | M_ERRNO, "%s: TCP port write failed on send()", __func__);
             goto error;
         }
     }
@@ -424,6 +423,11 @@ 
     {
         goto error;
     }
+    if (!relay_addr->addr.in4.sin_addr.s_addr)
+    {
+        msg(D_LINK_ERRORS, "%s: Socks proxy did not answer with IPv4 address", __func__);
+        goto error;
+    }
 
     return;