[Openvpn-devel,v2] socks.c: fix alen for DOMAIN type addresses

Message ID 20200909120707.8663-1-gert@greenie.muc.de
State Superseded
Headers show
Series
  • [Openvpn-devel,v2] socks.c: fix alen for DOMAIN type addresses
Related show

Commit Message

Gert Doering Sept. 9, 2020, 12:07 p.m.
When a SOCKS5 server sends back a reply, it encodes an "address",
which can be IPv4 (4 bytes), IPv6 (16 bytes) or "a domain name",
which has a lenght (1 byte) and "a string of length <length>" - so
when copying bytes, we need to hande "length +1" bytes.

Our code totally doesn't use this variant of addresses, but since
this has been pointed out by "tpw_rules" in Trac, fix it, so if/when
someone works on this again, the foundation is correct.

v2: increase buf[] len to be large enough to actually copy a domain name

Reported-By: tpw_rules in Trac
Trac: #848

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 src/openvpn/socks.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Patch

diff --git a/src/openvpn/socks.c b/src/openvpn/socks.c
index 57f0cee2..f1bdf4d4 100644
--- a/src/openvpn/socks.c
+++ b/src/openvpn/socks.c
@@ -312,7 +312,7 @@  recv_socks_reply(socket_descriptor_t sd,
     char atyp = '\0';
     int alen = 0;
     int len = 0;
-    char buf[22];
+    char buf[270];		/* 4 + alen(max 256) + 2 */
     const int timeout_sec = 5;
 
     if (addr != NULL)
@@ -381,7 +381,10 @@  recv_socks_reply(socket_descriptor_t sd,
                     break;
 
                 case '\x03':    /* DOMAINNAME */
-                    alen = (unsigned char) c;
+                    /* RFC 1928, section 5: 1 byte length, <n> bytes name,
+                     * so the total "address length" is (length+1)
+                     */
+                    alen = (unsigned char) c +1;
                     break;
 
                 case '\x04':    /* IP V6 */