Index | Thread | Search

From:
spiros thanasoulas <dsp@2f30.org>
Subject:
[Patch] nc(1) strncmp in socks mode is using the wrong length
To:
tech@openbsd.org
Date:
Thu, 15 May 2025 15:14:10 -0600

Download raw body.

Thread
  • spiros thanasoulas:

    [Patch] nc(1) strncmp in socks mode is using the wrong length

Hello list!

I am attaching a patch that fixes the parsing of the HTTP response from 
a socks proxy to properly match the HTTP 200 or HTTP 407 modes. As the
strncmp call is right now it would match a response of (for example)
HTTP/1.0 407123 as something that belongs to the code path of the 
HTTP/1.0 407 case. However I do not believe that this has any impact.

A bit unrelated to this patch but i found this using the tool 
https://github.com/disconnect3d/cstrnfinder
which flags size mismatches in operands.

There are a couple more places in the tree that this pattern appears
like (not the complete list)

/usr/src/gnu/gcc/gcc/config/darwin.c: 
else if (!strncmp(name, "_objc_category_class_methods_", 20))   strlen=29, n=20

/usr/src/gnu/gcc/gcc/config/darwin.c:
else if (!strncmp(name, "_objc_category_instance_methods_", 23))   strlen=32, n=23

/usr/src/gnu/gcc/gcc/config/m32c/m32c.c:
if (memcmp(str, "rpa", 2) == 0)   strlen=3, n=2

the last one which seems to be in our version of gcc has been fixed
upstream and i believe it might have some impact cause it relates to 
the machine constraints of that platform and could cause confusion on
how a register is treated when generating code for the m32c platform
(https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html)
granted i don't know how many people are compiling m32c code on obsd.

If you think we care about them i can work on individual patches.

? nc-strncpy.diff
Index: socks.c
===================================================================
RCS file: /cvs/src/usr.bin/nc/socks.c,v
retrieving revision 1.31
diff -u -p -r1.31 socks.c
--- socks.c	8 Jun 2022 20:20:26 -0000	1.31
+++ socks.c	15 May 2025 20:50:25 -0000
@@ -373,16 +373,16 @@ socks_connect(const char *host, const ch
 		/* Read status reply */
 		proxy_read_line(proxyfd, buf, sizeof(buf));
 		if (proxyuser != NULL &&
-		    (strncmp(buf, "HTTP/1.0 407 ", 12) == 0 ||
-		    strncmp(buf, "HTTP/1.1 407 ", 12) == 0)) {
+		    (strncmp(buf, "HTTP/1.0 407 ", 13) == 0 ||
+		    strncmp(buf, "HTTP/1.1 407 ", 13) == 0)) {
 			if (authretry > 1) {
 				fprintf(stderr, "Proxy authentication "
 				    "failed\n");
 			}
 			close(proxyfd);
 			goto again;
-		} else if (strncmp(buf, "HTTP/1.0 200 ", 12) != 0 &&
-		    strncmp(buf, "HTTP/1.1 200 ", 12) != 0)
+		} else if (strncmp(buf, "HTTP/1.0 200 ", 13) != 0 &&
+		    strncmp(buf, "HTTP/1.1 200 ", 13) != 0)
 			errx(1, "Proxy error: \"%s\"", buf);
 
 		/* Headers continue until we hit an empty line */