in order to prevent future bugs like the one fixed in cc7bc891ff
we need to assure that the response is of the same type as the request -
if not, some unexpected race condition happened.
it was reported that weechat 2.0 on ubuntu 16.04 LTS x86_64 segfaulted like this:
4 0x00007f6bf0e7e0c0 in __stack_chk_fail () at stack_chk_fail.c:28
5 0x00007f6bf2536bce in at_get_ip_for_host (host=0x339c4d0 "abcdefghijklmnop.onion", len=22) at src/allocator_thread.c:290
readbuf = {octet = "irc.", as_int = 778269289} msg = {msgtype = ATM_GETNAME, datalen = 13}
what happened was that weechat forked, thus got its own private copy of the VM
and thus a private copy of the mutex which should prevent parallel use of
at_get_ip_for_host() & friends. therefore the following race was possible:
- process A writes a message of type ATM_GETIP into the server pipe
- process B writes a message of type ATM_GETNAME into the server pipe
- process A write transaction is finished, and goes into receive mode
- server thread reads process B's message and responds with a ATM_GETNAME msg
- process A reads the response which was intended for process B into the 4 byte
ip address buffer, but ATM_GETNAME are much larger than ATM_GETIP responses,
resulting in stack corruption.
to prevent this issue, the storage of the mutex must reside in shared memory,
which we achieve via mmap. alternatively, shm_open() or sysvipc shm stuff could
be used. the former requires the mmap call to happen before the fork, the latter
not, however the shm would require a named object in /dev/shm (which requires
generating a unique name per proxychains instance, and subsequent cleanup).
so in the end, the mmap is easier to deal with, and we can be reasonably
certain that our constructor is being run before the hooked application forks.
it turned out that calling dlsym() may call malloc() in turn,
so we end up with the same deadlock described in the latest commit.
we thus now put all the fds passed to close pre-init into a list
and close them at init time.
this may finally fix#119.
it was observed that it is a bad idea to initialize the entire
infrastructure used by proxychains from the close hook,
because the following scenario will lead to a deadlock:
- it is possible that the dynlinker executes the initializer code of
other shared libs first
- if that code directly or indirectly calls malloc()
- which calls close() if it decided to use an mmap based allocation
- will now call our close(), which does
- call pthread_once which requires a lock
- creates a thread which calls malloc()
- which in turn calls our close() another time
- and our close is still in locked state.
so it seems the only save thing to do is to just get the address
of the original close function, and call that when we're in a
pre-init state.
this may hold for other functions that do lazy initialization as well,
however for those just calling the original function is probably
undesired since that could result in unproxified connections.
it will be needed to analyze on a per-function basis what the best
thing to do is, and finally rely only on the execution of the init
function from the gcc initializer.
should fix#119
hostsreader_get() used to assign the IP address to both `name` and `ip`
fields in `struct hostsreader`, which led to proxychains effectively
ignoring the contents of /etc/hosts.
if an ipv4-mapped ipv6 address is detected, the ip is converted
into v4 format because it may actually be one of our remote dns ips.
it was reported that a program called "maven", when getting handed our
fake ips in the remote dns subnet, converts the ip to v6 prior to calling
connect():
[proxychains] Strict chain ... 127.0.0.1:1080 ... ::ffff:224.0.0.1:443
<--socket error or timeout!
fixes#77
only basic testing was done (with 2 socks5 proxies listening on ::1)
but seems to work as intended.
ipv6 support for the hostsreader (/etc/hosts) is not implemented so far.
since "user" always points to a statically allocated string buffer,
the test for if(user)... was bogus.
use ulen instead.
this bug should only be visible on socks servers that require auth
if username was not passed, so it was probably not really an issue.
the allocatorthread got pointers to RAM which were reallocated
behind the back, and if realloc() couldn't grow in-place, lead
to segfaults in applications that do a lot of DNS-lookups such
as webbrowsers.
closes#66closes#31
thanks to @ravomavain for tracking down the issue.
working directly with the passed variables could lead to bugs when
some lines in the hosts file aren't well-formed and the loop is taken
several times while the buf vars are already modified.
the hostentdb introduced between 4.2 and 4.3
(via af5c6f0c6a )
had several issues:
- it caused breakage on FreeBSD and was commented out there
- prevented usage of the hostdb when proxy_dns was turned off
(issue #42)
- required dynamic memory allocation which was accessed from several
threads
- wouldnt reflect changes to the hosts file made during program run
the only sensible solution is to remove the hostentdb and replace it
with a home-grown hosts parser (we can't use gethostent() since
that would mess up the gethostent()-state from different threads).
the new parser used here is deliberately held simple and only meant
to provide the user with means to reference hardcoded ipv4 addresses
via his hosts file.
fixes#42
