Details
-
Bug
-
Status: Open
-
Trivial
-
Resolution: Unresolved
-
1.14.0
Description
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),
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
The test which follows is currently missing PY decrefs.
#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();
}