From 0e0d52815b75a12b9fbce86a124ce3495e16e484 Mon Sep 17 00:00:00 2001 From: Selva Nair Date: Sun, 9 Feb 2020 16:08:12 -0500 Subject: [PATCH] Re-queue reading from service only after previous message is handled The current code re-issues the next read request in the I/O completion routine before the previous message is fully handled. This could potentially lead to lost messages as the message buffer is reused. Fix by re-queuing the next read from OnService() after duplicating the previous message. The length check of the read message is omitted as it is implicitly checked when scanning the message. Makes the logic simpler. Reported by Lev Stipakov Signed-off-by: Selva Nair --- openvpn.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/openvpn.c b/openvpn.c index 64527e9..f0c4c8b 100644 --- a/openvpn.c +++ b/openvpn.c @@ -1343,7 +1343,7 @@ InitServiceIO (service_io_t *s) /* * Read-completion routine for interactive service pipe. Call with - * err = 0, bytes = 0 to queue the first read request. + * err = 0, bytes = 0 to queue a new read request. */ static void WINAPI HandleServiceIO (DWORD err, DWORD bytes, LPOVERLAPPED lpo) @@ -1360,6 +1360,7 @@ HandleServiceIO (DWORD err, DWORD bytes, LPOVERLAPPED lpo) int nchars = bytes/sizeof(s->readbuf[0]); s->readbuf[nchars] = L'\0'; SetEvent (s->hEvent); + return; } if (err) { @@ -1369,7 +1370,7 @@ HandleServiceIO (DWORD err, DWORD bytes, LPOVERLAPPED lpo) return; } - /* queue next read request */ + /* Otherwise queue next read request */ ReadFileEx (s->pipe, s->readbuf, capacity, lpo, HandleServiceIO); /* Any error in the above call will get checked in next round */ } @@ -1415,12 +1416,16 @@ OnService(connection_t *c, UNUSED char *msg) DWORD err = 0; DWORD pid = 0; WCHAR *p, *buf, *next; - DWORD len; const WCHAR *prefix = L"IService> "; - len = wcslen (c->iserv.readbuf); - if (!len || (buf = wcsdup (c->iserv.readbuf)) == NULL) - return; + /* + * Duplicate the read buffer and queue the next read request + * by calling HandleServiceIO with err = 0, bytes = 0. + */ + buf = wcsdup(c->iserv.readbuf); + HandleServiceIO(0, 0, (LPOVERLAPPED) &c->iserv); + + if (buf == NULL) return; /* messages from the service are in the format "0x08x\n%s\n%s" */ if (swscanf (buf, L"0x%08x\n", &err) != 1)