Segmentation fault if signal received during address resolution

[MacGyver]

If an interrupt signal is sent to the openvpn process during name resolution there is a chance that the process will crash with a segmentation fault instead of exiting cleanly.

Affected source files: socket.c, observed in 2.3.0 and in master. Lines 1264-1289 and lines 260-272 at the moment of writing.

Cause: If a signal (SIGINT) is received during name resolution in openvpn_getaddrinfo(...) called in resolve_remote(...), the address resolution fails and *ai is filled with bogus data. However, openvpn_getaddrinfo still returns 0 on exit, prompting a copy of ai->ai_addr - signal checking is done *after* this potential copy, not before. Since the pointer is bogus data, odds are that it's outside of memory bounds and therefore will cause a segmentation fault before openvpn has a chance to handle the SIGINT.

[Samuli Seppänen]

I can see that this could be quite annoying if name resolution is slow or does not work at all, and you'd need to kill the OpenVPN daemon. Does the segmentation fault happen for every type of signal (see man openvpn) or just SIGINT/SIGTERM?

[Samuli Seppänen]

This seems somewhat related to #311.

[Samuli Seppänen]

Plaisthos' dual-stack patches fix this issue. They will be merged into Git master shortly.

[Samuli Seppänen]

Fixed in Git "master". The 2.3 branch still has this issue.

[Gert Döring]

I *think* it might actually be these beauty here, in openvpn_getaddrinfo()

if (*signal_received == SIGUSR1) /* ignore SIGUSR1 */

{

msg (level, "RESOLVE: Ignored SIGUSR1 signal received duri

ng DNS resolution attempt");

*signal_received = 0;

}

else

{

if (0 == status) {

ASSERT(res);
freeaddrinfo(*res);
res = NULL;

}
goto done;

... which, if a signal is received while in getaddrinfo() *but* getaddrinfo() succeeds anyway (to reproduce: put a non-reachable nameserver first in resolv.conf, a working one second, switchover takes a few seconds, kill -INT in between), it will actually destroy the returned data AND fail to null the pointer, so the caller gets pointed to memory that might or might not still have valid data.

On my FreeBSDs, I cannot reproduce the crash, but if I change the code to read "*res = NULL", it will nicely crash in resolve_remote(), trying to access ai->ai_addr - as it should be, if the data has been freeaddrinfo()ed...

So, enough talking to myself, I *think* a proper fix might look like this (for release/2.3, with some fuzz due to extra debugging, but should apply to 2.3.6 as well - there has been a code change for 2.3.4, not actually fixing the problem but hiding it better - 47a963681a6ed):

--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -219,10 +219,13 @@ msg(M_INFO, "if(*signal_received)");

}

else

{

+ /* turn success into failure (interrupted syscall) */

if (0 == status) {

ASSERT(res);
freeaddrinfo(*res);

• res = NULL;

+ *res = NULL;
+ status = EAI_SYSTEM;
+ errno = EINTR;

}
goto done;

}

(not only fixing the spurious bug, but also introducing generall well-behaviour here - if we freeadrinfo(*res) we should no longer claim "success!")

Plaisthos, what do you think?

MacGyver?: can you reproduce the actual crash in your environment, and if yes, could you give this patch a try?

(As a side note: this code is also in git master, but as the surrounding code is different, it might be harder to trigger now - but should be fixed anyway)

[Gert Döring]

patch as described

[Gert Döring]

Patch has been applied to the master and release/2.3 branch.

commit 5f6c01ea6172ed1d8ed04e31f9f6c3f8e4696109 (master)
commit 38c9f980d4f7cb7061f9db3ed8645ab3404e533d (release/2.3)

Author: Gert Doering
Date: Sun May 31 22:41:58 2015 +0200

On signal reception, return EAI_SYSTEM from openvpn_getaddrinfo().

Signed-off-by: Gert Doering <gert@…>
Acked-by: Arne Schwabe <arne@…>
Message-Id: <1433104918-9523-1-git-send-email-gert@…>
URL: ​http://article.gmane.org/gmane.network.openvpn.devel/9764

I'm confident that this is the right thing, and Plaisthos ACKed it :-) - so I'm closing this ticket. MacGyver?: if you can still reproduce the crashes, I have overlooked something (obviously), so please reopen.