[4252] CID 1364685. Unchecked dynamic_cast in Option::cloneInternal.

diff --git a/src/lib/dhcp/option.h b/src/lib/dhcp/option.h
index 6f53fcf843c37970c807b5829eaa80bc1438bab6..3d0868c808f85870a9f60680e75e76e98725fb48 100644 (file)
--- a/src/lib/dhcp/option.h
+++ b/src/lib/dhcp/option.h
@@ -409,9 +409,11 @@ protected:
     /// be created.
     template<typename OptionType>
     OptionPtr cloneInternal() const {
-        boost::shared_ptr<OptionType>
-            option(new OptionType(*dynamic_cast<const OptionType*>(this)));
-        return (option);
+        const OptionType* cast_this = dynamic_cast<const OptionType*>(this);
+        if (cast_this) {
+            return (boost::shared_ptr<OptionType>(new OptionType(*cast_this)));
+        }
+        return (OptionPtr());
     }
 
     /// @brief Store option's header in a buffer.

diff --git a/src/lib/dhcp/tests/option_unittest.cc b/src/lib/dhcp/tests/option_unittest.cc
index 3d8289d47fcfb3eea809da175e3db3220f128e9b..247a53d52aa60f675ee427b8532fd94c4a1f4f32 100644 (file)
--- a/src/lib/dhcp/tests/option_unittest.cc
+++ b/src/lib/dhcp/tests/option_unittest.cc
@@ -9,6 +9,7 @@
 #include <dhcp/dhcp6.h>
 #include <dhcp/libdhcp++.h>
 #include <dhcp/option.h>
+#include <dhcp/option_int.h>
 #include <exceptions/exceptions.h>
 #include <util/buffer.h>
 
@@ -40,6 +41,7 @@ public:
     }
 
     using Option::unpackOptions;
+    using Option::cloneInternal;
 };
 
 class OptionTest : public ::testing::Test {
@@ -602,4 +604,18 @@ TEST_F(OptionTest, setEncapsulatedSpace) {
 
 }
 
+// This test verifies that cloneInternal returns NULL pointer if
+// non-compatible type is used as a template argument.
+// By non-compatible it is meant that the option instance doesn't
+// dynamic_cast to the type specified as template argument.
+// In our case, the NakedOption doesn't cast to OptionUint8 as the
+// latter is not derived from NakedOption.
+TEST_F(OptionTest, cloneInternal) {
+    NakedOption option;
+    OptionPtr clone;
+    // This shouldn't throw nor cause segmentation fault.
+    ASSERT_NO_THROW(clone = option.cloneInternal<OptionUint8>());
+    EXPECT_FALSE(clone);
+}
+
 }