Module: check_mk
Branch: master
Commit: 1f6b36b778e98699982de7e464915a66f286cf3b
URL:
http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=1f6b36b778e986…
Author: Jukka Aro <ja(a)mathias-kettner.de>
Date: Thu Mar 15 09:14:27 2018 +0100
5919 FIX Windows agent crashed with a lot of transport protocols available
Werk #5704 introduced querying the supported IP families (IPv4/IPv6)
upon agent start. This query makes use of the WinAPI function
WSCEnumProtocols. This function turned out to contain a severe bug:
if the protocol buffer passed as in/out parameter is not big enough
to accommodate all available transport protocols, the function corrupts
the heap by writing to memory past the buffer. This violates the
MSDN documentation of WSCEnumProtocols, also the example code attached
to the documentation is broken.
So far, the bug has only been reported on older 32 bit systems
(Windows Server 2008). However, the real extent of the problem is not
known as there are no bug reports about WSCEnumProtocols publicly
available to determine, if the function is broken in all Windows
versions or just in some. A key factor is the number of
configured/supported transport protocols: if the number grows large
enough, WSCEnumProtocols causes the described buffer overflow.
Now the use of WSCEnumProtocols has been changed so that - on the
contrary to the documentation in MSDN - the function is always called
twice to prevent the buffer overflow by allocating the necessary buffer
only after first querying the necessary buffer size.
---
.werks/5919 | 32 ++++++++++++++++++++++++++++++++
agents/windows/build_version | 2 +-
agents/windows/check_mk_agent.cc | 20 +++++++++++---------
3 files changed, 44 insertions(+), 10 deletions(-)
diff --git a/.werks/5919 b/.werks/5919
new file mode 100644
index 0000000..89db666
--- /dev/null
+++ b/.werks/5919
@@ -0,0 +1,32 @@
+Title: Windows agent crashed with a lot of transport protocols available
+Level: 1
+Component: checks
+Compatible: compat
+Edition: cre
+Version: 1.5.0i4
+Date: 1521100568
+Class: fix
+
+Werk #5704 introduced querying the supported IP families (IPv4/IPv6)
+upon agent start. This query makes use of the WinAPI function
+WSCEnumProtocols. This function turned out to contain a severe bug:
+if the protocol buffer passed as in/out parameter is not big enough
+to accommodate all available transport protocols, the function corrupts
+the heap by writing to memory past the buffer. This violates the
+MSDN documentation of WSCEnumProtocols, also the example code attached
+to the documentation is broken.
+
+So far, the bug has only been reported on older 32 bit systems
+(Windows Server 2008). However, the real extent of the problem is not
+known as there are no bug reports about WSCEnumProtocols publicly
+available to determine, if the function is broken in all Windows
+versions or just in some. A key factor is the number of
+configured/supported transport protocols: if the number grows large
+enough, WSCEnumProtocols causes the described buffer overflow.
+
+Now the use of WSCEnumProtocols has been changed so that - on the
+contrary to the documentation in MSDN - the function is always called
+twice to prevent the buffer overflow by allocating the necessary buffer
+only after first querying the necessary buffer size.
+
+
diff --git a/agents/windows/build_version b/agents/windows/build_version
index 8aae935..38c07f5 100644
--- a/agents/windows/build_version
+++ b/agents/windows/build_version
@@ -1 +1 @@
-3174
+3176
diff --git a/agents/windows/check_mk_agent.cc b/agents/windows/check_mk_agent.cc
index fd3fa4a..44ed890 100644
--- a/agents/windows/check_mk_agent.cc
+++ b/agents/windows/check_mk_agent.cc
@@ -149,8 +149,6 @@ void RunImmediate(const char *mode, int argc, char **argv);
// '----------------------------------------------------------------------'
namespace {
-const DWORD DEFAULT_BUFFER_SIZE = 16384L;
-
class UnpackError : public std::runtime_error {
public:
UnpackError(const std::string &what) : std::runtime_error(what) {}
@@ -171,20 +169,24 @@ class MillisecondsFormatter : public Formatter {
// service stuff. At least, let us *not* make this yet another global variable
// and access them from other compilation units...
const WinApi s_winapi;
-
bool supportIPv6() {
INT iNuminfo = 0;
- DWORD bufferSize = DEFAULT_BUFFER_SIZE;
- std::vector<BYTE> protocolInfo(bufferSize, 0);
- int iErrno = 0;
- auto lpProtocolInfo =
- reinterpret_cast<LPWSAPROTOCOL_INFOW>(protocolInfo.data());
-
+ DWORD bufferSize = 0;
+ std::vector<BYTE> protocolInfo;
+ INT iErrno = NO_ERROR;
+ LPWSAPROTOCOL_INFOW lpProtocolInfo = nullptr;
+
+ // WSCEnumProtocols is broken (nice!). You *must* call it 1st time with null
+ // buffer & bufferSize 0. Otherwise it will corrupt your heap in case the
+ // necessary buffer size exceeds your allocated buffer. Do never ever trust
+ // Microsoft WinAPI documentation!
while ((iNuminfo = s_winapi.WSCEnumProtocols(nullptr, lpProtocolInfo,
&bufferSize, &iErrno)) ==
SOCKET_ERROR) {
if (iErrno == WSAENOBUFS) {
protocolInfo.resize(bufferSize, 0);
+ lpProtocolInfo =
+ reinterpret_cast<LPWSAPROTOCOL_INFOW>(protocolInfo.data());
} else {
std::cerr << "WSCEnumProtocols failed with error: " <<
iErrno
<< std::endl;