20 Apr
2018
20 Apr
'18
6:50 a.m.
Module: check_mk
Branch: master
Commit: 372a17b09ee44cd98a633b0cd1284eb39d15fc7a
URL:
http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=372a17b09ee44c…
Author: Jukka Aro <ja(a)mathias-kettner.de>
Date: Thu Apr 19 09:04:18 2018 +0200
5922 FIX Windows agent: Many WMI queries could lead to integer overflow
Integer values returned by a number of WMI queries were handled incorrectly,
squeezing 64 bit integers into 32 bit representation. This led to sporadical
erroneous results for a number of agent sections / checks. Potentially
affected were at least the checks wmi_cpuload and ps, but basically any checks
using information provided by WMI. The symptoms of this may have been varying
across different checks.
---
.werks/5922 | 17 +++++++++++++++++
agents/windows/build_version | 2 +-
agents/windows/it/test_section_dotnet_clrmemory.py | 2 +-
agents/windows/it/test_section_wmi_cpuload.py | 4 ++--
agents/windows/sections/SectionPS.cc | 14 +++++++-------
agents/windows/sections/SectionPS.h | 8 ++++----
agents/windows/wmiHelper.cc | 16 +++++-----------
agents/windows/wmiHelper.h | 2 +-
8 files changed, 38 insertions(+), 27 deletions(-)
diff --git a/.werks/5922 b/.werks/5922
new file mode 100644
index 0000000..73cbce9
--- /dev/null
+++ b/.werks/5922
@@ -0,0 +1,17 @@
+Title: Windows agent: Many WMI queries could lead to integer overflow
+Level: 1
+Component: checks
+Compatible: compat
+Edition: cre
+Version: 1.6.0i1
+Date: 1524205799
+Class: fix
+
+Integer values returned by a number of WMI queries were handled incorrectly,
+squeezing 64 bit integers into 32 bit representation. This lead to sporadical
+erroneous results for a number of agent sections / checks. Potentially
+affected were at least the checks wmi_cpuload and ps, but basically any checks
+using information provided by WMI. The symptoms of this may have been varying
+across different checks.
+
+
diff --git a/agents/windows/build_version b/agents/windows/build_version
index 362875e..c5f1662 100644
--- a/agents/windows/build_version
+++ b/agents/windows/build_version
@@ -1 +1 @@
-3248
+3250
diff --git a/agents/windows/it/test_section_dotnet_clrmemory.py
b/agents/windows/it/test_section_dotnet_clrmemory.py
index 84e45b1..2a34c56 100644
--- a/agents/windows/it/test_section_dotnet_clrmemory.py
+++ b/agents/windows/it/test_section_dotnet_clrmemory.py
@@ -38,7 +38,7 @@ def expected_output():
],
repeat(r'\d+,,,\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,'
r'[\w\#\.]+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,'
- r'\d+,\-?\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+'))
+ r'\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+'))
def test_section_dotnet_clrmemory(testconfig, expected_output, actual_output,
diff --git a/agents/windows/it/test_section_wmi_cpuload.py
b/agents/windows/it/test_section_wmi_cpuload.py
index ca55544..5889ba6 100644
--- a/agents/windows/it/test_section_wmi_cpuload.py
+++ b/agents/windows/it/test_section_wmi_cpuload.py
@@ -34,7 +34,7 @@ def expected_output():
r'ProcessorQueueLength,SystemCallsPersec,SystemUpTime,Threads,'
r'Timestamp_Object,Timestamp_PerfTime,Timestamp_Sys100NS'),
(r'\d+,,\d+,,\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,,\d+,'
- r'\-?\d+,\d+,\d+,\-?\d+,\d+,\d+,\d+,\d+,\d+'),
+ r'\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+,\d+'),
re.escape(r'[computer_system]'),
(r'AdminPasswordStatus,AutomaticManagedPagefile,'
r'AutomaticResetBootOption,AutomaticResetCapability,BootOptionOnLimit,'
@@ -57,7 +57,7 @@ def expected_output():
(r'\d+,\d+,\d+,\d+,\d*,\d*,\d+,[^,]*,[^,]+,[\w-]+,\d+,,\w+,\d+,\d+,'
r'[^,]+,[\w-]+,[^,]+,\d+,\d+,\d+,\d+,\d+,,,\d+,,[^,]+(, [^,]+)?,[^,]+,'
r'[\w-]+,,\d+,\d+,\d+,,[^,]+,\d+,\-?\d+,\d+,\d+,,,\d+,\d+,\d+,,[\w-]+,'
- r'\d+,\-?\d+,\-?\d+,[^,]+,\w+,,[^,]*,,,,,[^,]+,\d+,\d+,[^,]*,\d+,\w*')
+ r'\d+,\d+,\d+,[^,]+,\w+,,[^,]*,,,,,[^,]+,\d+,\d+,[^,]*,\d+,\w*')
]
diff --git a/agents/windows/sections/SectionPS.cc b/agents/windows/sections/SectionPS.cc
index 13a1e97..352aacb 100644
--- a/agents/windows/sections/SectionPS.cc
+++ b/agents/windows/sections/SectionPS.cc
@@ -160,9 +160,9 @@ bool SectionPS::produceOutputInner(std::ostream &out,
void SectionPS::outputProcess(
std::ostream &out, ULONGLONG virtual_size, ULONGLONG working_set_size,
- ULONGLONG pagefile_usage, ULONGLONG uptime, ULONGLONG usermode_time,
- ULONGLONG kernelmode_time, DWORD process_id, DWORD process_handle_count,
- DWORD thread_count, const std::string &user, const std::string &exe_file) {
+ long long pagefile_usage, ULONGLONG uptime, long long usermode_time,
+ long long kernelmode_time, long long process_id, long long process_handle_count,
+ long long thread_count, const std::string &user, const std::string &exe_file)
{
// Note: CPU utilization is determined out of usermodetime and
// kernelmodetime
out << "(" << user << "," << virtual_size
/ 1024 << ","
@@ -185,7 +185,7 @@ bool SectionPS::outputWMI(std::ostream &out) {
bool more = result.valid();
while (more) {
- int processId = result.get<int>(L"ProcessId");
+ long long processId = result.get<long long>(L"ProcessId");
NullHandle process(
_winapi.OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ,
@@ -224,11 +224,11 @@ bool SectionPS::outputWMI(std::ostream &out) {
outputProcess(
out,
std::stoull(result.get<std::string>(L"VirtualSize")),
std::stoull(result.get<std::string>(L"WorkingSetSize")),
- result.get<int>(L"PagefileUsage"), uptime,
+ result.get<long long>(L"PagefileUsage"), uptime,
std::stoull(result.get<std::wstring>(L"UserModeTime")),
std::stoull(result.get<std::wstring>(L"KernelModeTime")),
- processId, result.get<int>(L"HandleCount"),
- result.get<int>(L"ThreadCount"), user,
to_utf8(process_name));
+ processId, result.get<long long>(L"HandleCount"),
+ result.get<long long>(L"ThreadCount"), user,
to_utf8(process_name));
more = result.next();
}
diff --git a/agents/windows/sections/SectionPS.h b/agents/windows/sections/SectionPS.h
index 63a7766..174a215 100644
--- a/agents/windows/sections/SectionPS.h
+++ b/agents/windows/sections/SectionPS.h
@@ -59,10 +59,10 @@ private:
std::string &csOwner_o);
process_entry_t getProcessPerfdata();
void outputProcess(std::ostream &out, ULONGLONG virtual_size,
- ULONGLONG working_set_size, ULONGLONG pagefile_usage,
- ULONGLONG uptime, ULONGLONG usermode_time,
- ULONGLONG kernelmode_time, DWORD process_id,
- DWORD process_handle_count, DWORD thread_count,
+ ULONGLONG working_set_size, long long pagefile_usage,
+ ULONGLONG uptime, long long usermode_time,
+ long long kernelmode_time, long long process_id,
+ long long process_handle_count, long long thread_count,
const std::string &user, const std::string &exe_file);
bool outputWMI(std::ostream &out);
bool outputNative(std::ostream &out);
diff --git a/agents/windows/wmiHelper.cc b/agents/windows/wmiHelper.cc
index 67b4cfd..4e42f1f 100644
--- a/agents/windows/wmiHelper.cc
+++ b/agents/windows/wmiHelper.cc
@@ -254,20 +254,14 @@ bool Result::next() {
}
template <>
-int Variant::get() const {
+long long Variant::get() const {
switch (_value.vt) {
case VT_I1:
- return _value.bVal;
- case VT_I2:
return _value.iVal;
- case VT_I4:
- return _value.intVal;
- case VT_UI1:
- return _value.cVal;
- case VT_UI2:
- return _value.uiVal;
- case VT_UI4:
+ case VT_I2:
return _value.intVal;
+ case VT_I4:
+ return _value.llVal;
default:
throw ComTypeException(string("wrong value type requested: ") +
to_string(_value.vt));
@@ -367,7 +361,7 @@ wstring Variant::get() const {
case VT_I1:
case VT_I2:
case VT_I4:
- return std::to_wstring(get<int>());
+ return std::to_wstring(get<long long>());
case VT_UI1:
case VT_UI2:
case VT_UI4:
diff --git a/agents/windows/wmiHelper.h b/agents/windows/wmiHelper.h
index 19b3cec..cf80f83 100644
--- a/agents/windows/wmiHelper.h
+++ b/agents/windows/wmiHelper.h
@@ -79,7 +79,7 @@ private:
};
template <>
-int Variant::get() const;
+long long Variant::get() const;
template <>
bool Variant::get() const;
template <>