Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect IPv6/hostname/port displayed in SOCKS5 connection error #944

Closed
DonKult opened this issue Aug 6, 2016 · 7 comments
Closed

Incorrect IPv6/hostname/port displayed in SOCKS5 connection error #944

DonKult opened this issue Aug 6, 2016 · 7 comments

Comments

@DonKult
Copy link

DonKult commented Aug 6, 2016

While writing a socks5 client, testing the results & comparing it to other implementations

I did this

socat -x tcp-listen:2903,reuseaddr system:"echo -n '$CODE' | xxd -r -p"
with three different $CODEs being:

05 00 05 04 00 01 ac 10 fe 01 1f 90
05 00 05 04 00 03 09 68 6f 73 74 6c 6f 63 61 6c 1f 90
05 00 05 04 00 04 20 01 0d b8 ac 10 fe 00 00 00 00 00 00 00 00 00 1f 90

(aka: pretending to be a socks5 server with no auth and failing the request with IPv4, hostname & IPv6)
and each time: curl --proxy socks5h://localhost:2903 http://example.org

I expected the following

For the first (ipv4) code the result is correct:
curl: (7) Can't complete SOCKS5 connection to 172.16.254.1:8080. (4)
The hostname one should have been hostlocal:8080, but is:
curl: (7) Can't complete SOCKS5 connection to localhost:29804. (4)
The IPv6 should be [2001:0DB8:AC10:FE00:0000:0000:0000:0000]:8080, but is:
curl: (7) Can't complete SOCKS5 connection to 2001:0db8:ac10:686f:7374:1f90:0000:8000:44048. (4)

Looking at the source I think this len = 10; is the start of the issue at least for the IPv6 as that is "just" the right value for IPv4. The wrong port is the result of a copy&paste from IPv4 I presume and the wrong hostname is caused by displaying the hostname the request was made for completely discarding what the server reports as hostname it has connected to (which is likely the same – at least if your starting point was a hostname and not an ip –, but still). In case you wonder: Depending on the length of the request the code is using uninitialized memory for IPv6 (aka the later octets of the address), but I fail to imagine a way to exploit that…

The non-error case does deal with all three types correctly btw – that isn't very hard through as it is just discarding the data. I would have kinda expected a debug message here but nope.

So in summary: Worst bugs ever… if I would know how to set a minor severity on github issues I would do it.

[dropping the rest of the bug template as presumably not relevant]

@bagder
Copy link
Member

bagder commented Aug 6, 2016

I don't understand.

The hostname one should have been hostlocal:8080, but is:
curl: (7) Can't complete SOCKS5 connection to localhost:29804. (4)

Why "hostlocal" ? And did it really say 29804 when you connected to 2903? That seems really weird.

The IPv6 should be [2001:0DB8:AC10:FE00:0000:0000:0000:0000]:8080, but is:

Sorry, but why/how should it be that? And is this perhaps related to https://curl.haxx.se/docs/knownbugs.html#SOCKS_proxy_not_working_via_IPv6 ?

@DonKult
Copy link
Author

DonKult commented Aug 6, 2016

With the codes like 05 00 05 04 00 03 09 68 6f 73 74 6c 6f 63 61 6c 1f 90 I use socat to pretend to curl that it is talking to a socks5 proxy, which in this case is negotiating for no authorization and denies the connection with error code 4 to the server detailed in the rest of the reply. The 68 6f 73 74 6c 6f 63 61 6c is the hostname which is "hostlocal" in hex. I should probably have chosen a different hostname, but I was lazy by just moving octets around in testing. The two following octets defining the port are equally made up just for testing (I picked port 8080 just because). So, no real socks5 proxy is involved, I am just simulating what a socks5 would do to observe what the client does for testing.

Regarding port, just look at the source, for IPv6 its perhaps clearest: https://github.com/curl/curl/blob/master/lib/socks.c#L689
For all three types it chooses the 9th and 10th octet to calculate the port. That is correct for IPv4, but incorrect for hostname (logically, that can't be a fixed number given that the hostname is variable length and the port follows after the hostname) and for IPv6 the 9th and 10th octet are in the middle of the IPv6 number itself, so that logically can't really be producing the right port either.

These are all just display issues in error messages – no effect on successful runs and hence also unrelated to the known bugs – through as far as I know socks4 doesn't support IPv6 – one of the reasons for socks5…

I hope that makes it slightly clearer. Sorry for not being more explicit – having just recently read the relevant RFCs makes this all super obvious, but in a few days I would probably haven't understood myself either. ;)

mback2k added a commit that referenced this issue Aug 14, 2016
First commit to fix issue #944 regarding SOCKS5 error handling.

Reported-by: David Kalnischkies
mback2k added a commit that referenced this issue Aug 14, 2016
Second commit to fix issue #944 regarding SOCKS5 error handling.

Reported-by: David Kalnischkies
mback2k added a commit that referenced this issue Aug 14, 2016
Third commit to fix issue #944 regarding SOCKS5 error handling.

Reported-by: David Kalnischkies
@mback2k
Copy link
Member

mback2k commented Aug 14, 2016

@DonKult Except not using the returned hostname, the incomplete response and invalid port issues should be fixed by the 3 commits mentioned above. Please try the current master. I did not fix the hostname part, because I think validating that the server did not return a malicious combination of hostname and its length is not worth the effort for this error case. So curl will display the requested hostname instead of the returned hostname in this error case.

@mback2k mback2k self-assigned this Aug 14, 2016
@DonKult
Copy link
Author

DonKult commented Aug 14, 2016

(not tested the commits yet)
The thing with the hostname is that the hostname displayed is the one used in the request, not the hostname the server returned. Most of the time it will be the same, but lets imagine your socks-proxy does support SRV records, so that you will request example.org and e.g. (via SRV lookup) get back foobar.example.net. Nothing malicious about it, but if its worth to fix it or not is your call of course.

@mback2k
Copy link
Member

mback2k commented Aug 14, 2016

In any case it should probably return both the requested and the returned IP or hostname. That would probably be the most clearest solution.

@mback2k
Copy link
Member

mback2k commented Aug 20, 2016

@DonKult I just pushed a series of commits to the master branch in order to streamline the verbose debug and error messages during the SOCKS4 and SOCKS5 connection sequences. At least in case of an error in the final phase of the connection sequence, the destination (IPv4, hostname or IPv6) returned by the proxy server will be displayed. Please check if the issue can now be considered fixed.

@bagder
Copy link
Member

bagder commented Aug 28, 2016

Presumably fixed, closing

@bagder bagder closed this as completed Aug 28, 2016
@lock lock bot locked as resolved and limited conversation to collaborators May 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

3 participants