Merge pull request #382 from dnp3/bug/prevent-duration-overflow

check maximum values for ms, s, min when constructing TimeDuration

cpp/lib/include/opendnp3/util/TimeDuration.h

@@ -45,11 +45,7 @@ class TimeDuration
 
     static TimeDuration Seconds(int64_t seconds);
 
-    static TimeDuration Minutes(int64_t minutes);
-
-    static TimeDuration Hours(int64_t hours);
-
-    static TimeDuration Days(int64_t days);
+    static TimeDuration Minutes(int64_t minutes);    
 
     TimeDuration();
 
@@ -68,6 +64,8 @@ class TimeDuration
     std::chrono::steady_clock::duration value;
 
 private:
+    template<class T> static TimeDuration FromValue(int64_t value);
+
     explicit TimeDuration(std::chrono::steady_clock::duration value);
 };
 

cpp/lib/src/util/TimeDuration.cpp

@@ -39,31 +39,42 @@ TimeDuration TimeDuration::Zero()
     return TimeDuration(std::chrono::milliseconds(0));
 }
 
-TimeDuration TimeDuration::Milliseconds(int64_t milliseconds)
+template<class T> TimeDuration TimeDuration::FromValue(int64_t value)
 {
-    return TimeDuration(std::chrono::milliseconds(milliseconds));
-}
+    // > this will overflow when converting to nanos
+    const auto MAX = std::chrono::duration_cast<T>(std::chrono::steady_clock::duration::max()).count();
+    const auto MIN = std::chrono::duration_cast<T>(std::chrono::steady_clock::duration::min()).count();
 
-TimeDuration TimeDuration::Seconds(int64_t seconds)
-{
-    return TimeDuration(std::chrono::seconds(seconds));
+    if (value > MAX)
+    {
+        return TimeDuration(std::chrono::steady_clock::duration::max());
+    }
+
+    if (value < MIN)
+    {
+        return TimeDuration(std::chrono::steady_clock::duration::min());
+    }
+
+    return TimeDuration(T(value));
 }
 
-TimeDuration TimeDuration::Minutes(int64_t minutes)
-{
-    return TimeDuration(std::chrono::minutes(minutes));
+TimeDuration TimeDuration::Milliseconds(int64_t milliseconds)
+{        
+    return FromValue<std::chrono::milliseconds>(milliseconds);
 }
 
-TimeDuration TimeDuration::Hours(int64_t hours)
+
+TimeDuration TimeDuration::Seconds(int64_t seconds)
 {
-    return TimeDuration(std::chrono::hours(hours));
+    return FromValue<std::chrono::seconds>(seconds);
 }
 
-TimeDuration TimeDuration::Days(int64_t days)
+TimeDuration TimeDuration::Minutes(int64_t minutes)
 {
-    return TimeDuration(std::chrono::hours(24) * days);
+    return FromValue<std::chrono::minutes>(minutes);
 }
 
+
 TimeDuration::TimeDuration() : value(std::chrono::milliseconds(0)) {}
 
 TimeDuration TimeDuration::Double() const

cpp/tests/unit/CMakeLists.txt

@@ -54,6 +54,7 @@ set(unittests_src
     ./TestOutstationUnsolicitedResponses.cpp
     ./TestShiftableBuffer.cpp
 	./TestStaticDataMap.cpp
+    ./TestTimeDuration.cpp
     ./TestTransportLayer.cpp
     ./TestTypedCommandHeader.cpp
     ./TestUpdateBuilder.cpp

cpp/tests/unit/TestTimeDuration.cpp

@@ -0,0 +1,43 @@
+/*
+ * Copyright 2013-2020 Automatak, LLC
+ *
+ * Licensed to Green Energy Corp (www.greenenergycorp.com) and Automatak
+ * LLC (www.automatak.com) under one or more contributor license agreements.
+ * See the NOTICE file distributed with this work for additional information
+ * regarding copyright ownership. Green Energy Corp and Automatak LLC license
+ * this file to you under the Apache License, Version 2.0 (the "License"); you
+ * may not use this file except in compliance with the License. You may obtain
+ * a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include <opendnp3/util/TimeDuration.h>
+
+#include <catch.hpp>
+
+using namespace opendnp3;
+
+#define SUITE(name) "TimeDurationTestSuite - " name
+
+TEST_CASE(SUITE("constructing from maximum values are clamped to prevent overflow when converting to nanos"))
+{
+    REQUIRE(TimeDuration::Milliseconds(std::numeric_limits<int64_t>::max()) == TimeDuration::Max());
+    REQUIRE(TimeDuration::Seconds(std::numeric_limits<int64_t>::max()) == TimeDuration::Max());
+    REQUIRE(TimeDuration::Minutes(std::numeric_limits<int64_t>::max()) == TimeDuration::Max());
+}
+
+TEST_CASE(SUITE("constructing from minimum values are clamped to prevent overflow when converting to nanos"))
+{    
+    REQUIRE(TimeDuration::Milliseconds(std::numeric_limits<int64_t>::min()) == TimeDuration::Min());
+    REQUIRE(TimeDuration::Seconds(std::numeric_limits<int64_t>::min()) == TimeDuration::Min());
+    REQUIRE(TimeDuration::Minutes(std::numeric_limits<int64_t>::min()) == TimeDuration::Min());    
+}
+
+
+