fix(DataNode): segfault when unlink unmanaged

When an Unmanaged DataNode is unlinked (e.g. was temporary linked to print a JSON string), the method dereferences a nullptr which is UB. Added tests in unsafe.cpp. Fixes: #21 Change-Id: Ia6a621e0b9d5faad8c64368b3a2e344e52288e87
CESNET · Jun 5, 2024 · c5ecde0 · c5ecde0
1 parent d8bf2a8
commit c5ecde0
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 4 deletions.
diff --git a/src/DataNode.cpp b/src/DataNode.cpp
@@ -641,7 +641,7 @@ void DataNode::unlink()
 {
     handleLyTreeOperation(this, [this] () {
         lyd_unlink_tree(m_node);
-    }, OperationScope::JustThisNode, std::make_shared<internal_refcount>(m_refs->context));
+    }, OperationScope::JustThisNode, std::make_shared<internal_refcount>(m_refs ? m_refs->context : nullptr));
 }
 
 /**
@@ -676,7 +676,7 @@ void DataNode::unlinkWithSiblings()
 {
     handleLyTreeOperation(this, [this] {
             lyd_unlink_siblings(m_node);
-    }, OperationScope::AffectsFollowingSiblings, std::make_shared<internal_refcount>(m_refs->context));
+    }, OperationScope::AffectsFollowingSiblings, std::make_shared<internal_refcount>(m_refs ? m_refs->context : nullptr));
 }
 
 /**

diff --git a/tests/unsafe.cpp b/tests/unsafe.cpp
@@ -4,12 +4,13 @@
  * Written by Václav Kubernát <[email protected]>
  *
  * SPDX-License-Identifier: BSD-3-Clause
-*/
+ */
 
 #include <doctest/doctest.h>
 #include <libyang-cpp/Context.hpp>
 #include <libyang-cpp/Utils.hpp>
 #include <libyang/libyang.h>
+#include <libyang/tree_data.h>
 #include "example_schema.hpp"
 #include "test_vars.hpp"
 #include "utils/filesystem_path.hpp"
@@ -40,7 +41,6 @@ TEST_CASE("Unsafe methods")
             ctx_deleter.release();
 
             auto wrapped = libyang::createUnmanagedContext(ctx, ly_ctx_destroy);
-
         }
 
         DOCTEST_SUBCASE("No custom deleter")
@@ -111,6 +111,20 @@ TEST_CASE("Unsafe methods")
             // Both are still unmanaged, both are accessible.
             REQUIRE(wrapped.path() == "/example-schema:leafInt32");
             REQUIRE(anotherNodeWrapped.path() == "/example-schema:leafInt8");
+
+            DOCTEST_SUBCASE("no explicit unlink") { }
+
+            DOCTEST_SUBCASE("unlink an unmanaged node from an unmanaged node")
+            {
+                REQUIRE(wrapped.findPath("/example-schema:leafInt8"));
+                REQUIRE(anotherNodeWrapped.findPath("/example-schema:leafInt32"));
+
+                // After unlink they are not reachable from each other
+                anotherNodeWrapped.unlink();
+                REQUIRE(!wrapped.findPath("/example-schema:leafInt8"));
+                REQUIRE(!anotherNodeWrapped.findPath("/example-schema:leafInt32"));
+                lyd_free_all(anotherNode);
+            }
         }
 
         // You have a C++ managed node and you want to insert that into an unmanaged node.
@@ -123,6 +137,22 @@ TEST_CASE("Unsafe methods")
             // BOTH are now unmanaged, both are accessible.
             REQUIRE(wrapped.path() == "/example-schema:leafInt32");
             REQUIRE(anotherNodeWrapped.path() == "/example-schema:leafInt8");
+
+            DOCTEST_SUBCASE("no explicit unlink") { }
+
+            DOCTEST_SUBCASE("unlink a managed node from an unmanaged node")
+            {
+                REQUIRE(wrapped.findPath("/example-schema:leafInt8"));
+                REQUIRE(anotherNodeWrapped.findPath("/example-schema:leafInt32"));
+
+                // After unlink they are not reachable from each other
+                anotherNodeWrapped.unlink();
+                REQUIRE(!wrapped.findPath("/example-schema:leafInt8"));
+                REQUIRE(!anotherNodeWrapped.findPath("/example-schema:leafInt32"));
+
+                // this is still unmanaged and we need to delete it
+                lyd_free_all(anotherNode);
+            }
         }
 
         // You have a C++ managed node and you want to insert an unmanaged node into it.