Browse Source

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

Marcin Siodelski 8 years ago
parent
commit
1486d13c0d
2 changed files with 21 additions and 3 deletions
  1. 5 3
      src/lib/dhcp/option.h
  2. 16 0
      src/lib/dhcp/tests/option_unittest.cc

+ 5 - 3
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.

+ 16 - 0
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);
+}
+
 }