commit cd5b0b7e8e1a473262e4753e46e62d416a65649d
author Ankita Vilas Gawade <ankita.gawade@intel.com> Thu Jan 20 20:55:04 2022 +0000
committer Krzysztof Grobelny <krzysztof.grobelny@intel.com> Wed Feb 02 12:17:28 2022 +0000
tree d900185adfb475e71b1a7d38da62c2836dcd276c
parent a950e42ba579a3498face1654e58c94d9ff1cf9a

Add limit to Metric URIs to avoid buffer overflow

This commit  Limits the Number of Metrics to 150 URIs to avoid
overpopulating the Event to be Sent. This is Necessary to avoid Buffer
Overflow During Creation of Periodic MetricReport (RedfishEvent) via
Event Service Subscription. This Buffer overflow occurs as the max size
of buffer used to send Events is limited to 1024*64

This commit also removes max-reading-parameter as max-number-metrics
indirectly places constraint over number of reading parameters (which
makes them mutually exclusive).The parameter max-number-metrics provides
a better control over size of the metrics compared to
max-reading-parameter as it restricts the number of overall uris instead
of number of metrics.

Additional limit for appendLimit was added to ensure we can store all
metric values in persistent memory.

Tested:
 - Created Metric Report Defination by POST to
   /redfish/v1/TelemetryService/MetricReportDefinitions/
   with <150, =150 and >150 Metric Uris
   Successfully returned error for metric uris>150.
 - Observed no buffer overflow for Maximum of 150 Metric Uris.
 - Verified buffer Overflow by Subscribing to Metric Reports
   using SSE.
 - Test case Passed
   [ RUN      ] TestReportManager.failToAddReportWith
                MoreSensorParametersThanExpected
   [       OK ] TestReportManager.failToAddReportWith
                MoreSensorParametersThanExpected (10 ms)
   [ RUN      ] TestReportManager.failToAddReportWith
                MoreMetricsThanExpected
   [       OK ] TestReportManager.failToAddReportWith
                MoreMetricsThanExpected (20 ms)
   [ RUN      ] TestReportManager.failToAddReportWith
                AppendLimitGreaterThanMax
   [       OK ] TestReportManager.failToAddReportWith
                AppendLimitGreaterThanMax (20 ms)

Signed-off-by: Ankita Vilas Gawade <ankita.gawade@intel.com>
Change-Id: I113c15c7b1054364d827f39582c7d3fbe9495d12

src/report_manager.cpp

diff --git a/src/report_manager.cpp b/src/report_manager.cpp
index 7ac8537..8687316 100644
--- a/src/report_manager.cpp
+++ b/src/report_manager.cpp
@@ -127,7 +127,7 @@
 void ReportManager::verifyAddReport(
     const std::string& reportId, const std::string& reportName,
     const ReportingType reportingType, Milliseconds interval,
-    const ReportUpdates reportUpdates,
+    const ReportUpdates reportUpdates, const uint64_t appendLimit,
     const std::vector<LabeledMetricParameters>& readingParams)
 {
     if (reportingType == ReportingType::onChange)
@@ -144,14 +144,29 @@
             "Reached maximal report count");
     }
 
+    if (appendLimit > maxAppendLimit)
+    {
+        throw sdbusplus::exception::SdBusError(
+            static_cast<int>(std::errc::invalid_argument),
+            "Append limit out of range");
+    }
+
     if (reportingType == ReportingType::periodic && interval < minInterval)
     {
         throw sdbusplus::exception::SdBusError(
             static_cast<int>(std::errc::invalid_argument), "Invalid interval");
     }
 
-    if (readingParams.size() > maxReadingParams)
+    size_t metricCount = 0;
+    for (auto metricParam : readingParams)
+    {
+        auto metricParamsVec =
+            metricParam.at_label<utils::tstring::SensorPath>();
+        metricCount += metricParamsVec.size();
+    }
 
+    if (readingParams.size() > maxNumberMetrics ||
+        metricCount > maxNumberMetrics)
     {
         throw sdbusplus::exception::SdBusError(
             static_cast<int>(std::errc::argument_list_too_long),
@@ -205,7 +220,7 @@
                                         existingReportIds, maxReportIdLength);
 
     verifyAddReport(id, name, reportingType, interval, reportUpdates,
-                    labeledMetricParams);
+                    appendLimit, labeledMetricParams);
 
     reports.emplace_back(reportFactory->make(
         id, name, reportingType, reportActions, interval, appendLimit,