[jira] [Created] (DISPATCH-1821) Double-free in qd_entity_configure_policy on error

["Jiri Daněk (Jira)" <ji...@apache.org>]

Jiri Daněk created DISPATCH-1821:
------------------------------------

             Summary: Double-free in qd_entity_configure_policy on error
                 Key: DISPATCH-1821
                 URL: https://issues.apache.org/jira/browse/DISPATCH-1821
             Project: Qpid Dispatch
          Issue Type: Bug
          Components: Policy Engine
    Affects Versions: 1.14.0
            Reporter: Jiri Daněk


As far as I can tell, this double-free is near impossible in normal operation of the router. Below, it is reproduced using a test, which passes in a value for that cannot be converted to a Python bool. There is no way for a legitimate user to make this happen.

Regarding a fix, I am not sure what the router should do when it detect invalid configuration. Freeing qd->policy is IMO not the right response, because that is normally freed in qd_dispatch_free. Printing an error and carying on (that now happens for {{maxConnections < 0}}) does not seem right to me either (but it's probably needed in case the config is changed through remote management interface),

{noformat}
2020-11-02 09:48:51.887312 +0100 ROUTER_CORE (info) In-process subscription L/$management
2020-11-02 09:48:51.939210 +0100 ERROR (error) Python: NotImplementedError: 
2020-11-02 09:48:51.941666 +0100 ERROR (error) Traceback (most recent call last):
  File "test_module", line 3, in __bool__
NotImplementedError

=================================================================
==27615==ERROR: AddressSanitizer: attempting double-free on 0x602000001810 in thread T0:
    #0 0x7f876d5191d7 in __interceptor_free (/nix/store/zhbhxp4jgalycwffildy2bwcgmrsmayk-gcc-9.2.0-lib/lib/libasan.so.5+0x10a1d7)
    #1 0x7f876cd33b95 in qd_policy_free ../src/policy.c:136
    #2 0x7f876cd347cf in qd_entity_configure_policy ../src/policy.c:178
    #3 0x4ef6a6 in _DOCTEST_ANON_FUNC_6 ../tests/c_unittests/test_policy.cpp:75
    #4 0x493edd in doctest::Context::run() ../tests/c_unittests/doctest.h:5849
    #5 0x4974e9 in main ../tests/c_unittests/doctest.h:5933
    #6 0x7f876b610d8a in __libc_start_main (/nix/store/xg6ilb9g9zhi2zg1dpi4zcp288rhnvns-glibc-2.30/lib/libc.so.6+0x23d8a)
    #7 0x43cd99 in _start (/home/jdanek/repos/qpid/qpid-dispatch/cmake-build-debug/tests/c_unittests/c_unittests+0x43cd99)

0x602000001810 is located 0 bytes inside of 5-byte region [0x602000001810,0x602000001815)
freed by thread T0 here:
    #0 0x7f876d5191d7 in __interceptor_free (/nix/store/zhbhxp4jgalycwffildy2bwcgmrsmayk-gcc-9.2.0-lib/lib/libasan.so.5+0x10a1d7)
    #1 0x7f876cd347c3 in qd_entity_configure_policy ../src/policy.c:177
    #2 0x4ef6a6 in _DOCTEST_ANON_FUNC_6 ../tests/c_unittests/test_policy.cpp:75
    #3 0x493edd in doctest::Context::run() ../tests/c_unittests/doctest.h:5849
    #4 0x4974e9 in main ../tests/c_unittests/doctest.h:5933
    #5 0x7f876b610d8a in __libc_start_main (/nix/store/xg6ilb9g9zhi2zg1dpi4zcp288rhnvns-glibc-2.30/lib/libc.so.6+0x23d8a)

previously allocated by thread T0 here:
    #0 0x7f876d4a40b5 in strdup (/nix/store/zhbhxp4jgalycwffildy2bwcgmrsmayk-gcc-9.2.0-lib/lib/libasan.so.5+0x950b5)
    #1 0x7f876cec8c99 in py_string_2_c ../src/python_utils.c:35
    #2 0x7f876cce21e2 in qd_entity_get_string ../src/entity.c:49
    #3 0x7f876cce2b5f in qd_entity_opt_string ../src/entity.c:84
    #4 0x7f876cd341b0 in qd_entity_configure_policy ../src/policy.c:161
    #5 0x4ef6a6 in _DOCTEST_ANON_FUNC_6 ../tests/c_unittests/test_policy.cpp:75
    #6 0x493edd in doctest::Context::run() ../tests/c_unittests/doctest.h:5849
    #7 0x4974e9 in main ../tests/c_unittests/doctest.h:5933
    #8 0x7f876b610d8a in __libc_start_main (/nix/store/xg6ilb9g9zhi2zg1dpi4zcp288rhnvns-glibc-2.30/lib/libc.so.6+0x23d8a)

SUMMARY: AddressSanitizer: double-free (/nix/store/zhbhxp4jgalycwffildy2bwcgmrsmayk-gcc-9.2.0-lib/lib/libasan.so.5+0x10a1d7) in __interceptor_free
==27615==ABORTING
Process finished with exit code 1
{noformat}

The test which follows is currently missing PY decrefs. 

{noformat}
#include <Python.h>

#include <thread>

#include "helpers.hpp"
#include "qdr_doctest.h"

extern "C" {
#include <qpid/dispatch/python_embedded.h>
#include <qpid/dispatch/router_core.h>
#include <router_core/router_core_private.h>
#include <terminus_private.h>

#include <inttypes.h>
#include <stdint.h>
#include <stdio.h>
}

TEST_CASE("policy") {
    std::thread([]() {
        WithNoMemoryLeaks leaks{};
        QDR               qdr;
        qdr.start();
        qdr.wait();

        {
            auto qd = qdr.qd;

            PyObject *module = Py_CompileString(
                // language: Python
                R"EOT(
class NotBool:
    def __bool__(self):
        raise NotImplementedError()


fake_policy = {
      "maxConnections": 4,
      "policyDir": "/tmp",
      "enableVhostPolicy": 4,
      "enableVhostNamePatterns": NotBool(),
})EOT",
                "test_module", Py_file_input);
            REQUIRE(module != nullptr);

            PyObject *pModuleObj = PyImport_ExecCodeModule("test_module", module);
            CHECK(pModuleObj != nullptr);
            PyErr_Print();

            PyObject *pAttrObj = PyObject_GetAttrString(pModuleObj, "fake_policy");
            REQUIRE(pAttrObj != nullptr);

            auto *entity = reinterpret_cast<qd_entity_t *>(pAttrObj);
            REQUIRE(qd_entity_has(entity, "enableVhostNamePatterns"));  // sanity check for the test input

            // TODO Py Decrefs probably needed somewhere

            const qd_python_lock_state_t lock_state = qd_python_lock();
            qd_error_t                   err = qd_dispatch_configure_policy(qd, entity);
            CHECK(err == QD_ERROR_PYTHON);
            qd_python_unlock(lock_state);
        }
        qdr.stop();
    }).join();
}
{noformat}




--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org