libimobiledevice / libimobiledevice-glue

A library with common code used by libraries and tools around the libimobiledevice project
GNU Lesser General Public License v2.1
91 stars 70 forks source link

Use of localhost in socket_create() of libimobiledevice-glue #40

Closed sctol closed 7 months ago

sctol commented 7 months ago

The use of "localhost" passed to getaddrinfo() in socket.c's socket_create() function will return zero or more addrinfo entries on Windows. Where both IPV6 and IPV4 are enabled on Windows in a default network configuration, the first entry will be AF_INET6 with address ::1 and the second will be AF_INET with address 127.0.0.1. If the first argument to socket_create() is NULL, the current socket.c code replaces this with "localhost" before calling getaddrinfo(). The code will then go on to bind and listen to the first successful sockaddr found. Since this would be family AF_INET6 on Windows, a call to socket_create(NULL, 27010) will create, bind, and listen a WS2 socket at ::1:27010 and return the socket. It will ignore creating a second one for AF_INET at 127.0.0.1::27010.

If a NULL is passed as the first argument of getaddrinfo(), it will also return a list of addrinfo entries, one for each loopback defined on enabled IPV6 or IPV4 stacks, same as using "localhost" does in an otherwise normal network, making the use of "localhost" unnecessary.

However, hard-coded use of "localhost" opens Libimobiledevice to the possibility of DNS rebind attacks (such as malicious modification of /etc/hosts, use of a compromised DNS server, or presence of a compromised DNS intercept service in a local router.) One of the DNS rebind attacks can also be avoided by using the anchored version "localhost." with the trailing period instead of the bare "localhost". The corresponding client-side function socket_connect() will return -1 if a NULL addr argument is passed in, So, in socket_create(), instead of replacing addr with "localhost" perhaps its use should be avoided entirely and make socket_create()'s address handling behavior the same as socket_connect()?

nikias commented 7 months ago

I changed it to allow NULL. Thanks for pointing this out. 9a2b733732b453c073cdd0e30e405ca84541f8e8