Skip to content

Commit

Permalink
Fix ResolverProxy use-after-free in HandleNodeBrowse
Browse files Browse the repository at this point in the history
HandleNodeBrowse decrements the ResolverProxy reference count, which
will cause the object to be destructed if the counter reaches 0, and
then increments the counter and uses the object, which can be a
use-after-free. This commit fixes the problem by ordering Release to
occur after Retain.

This commit also adds an abort to ReferenceCounted to check for cases
like this.  Calling Retain on an object with a reference count that has
already decremented to 0 is always a bug.

Fixes project-chip#13289
  • Loading branch information
msandstedt committed Dec 30, 2021
1 parent 8ba4702 commit 2e44597
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 5 deletions.
4 changes: 4 additions & 0 deletions src/lib/core/ReferenceCounted.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ class ReferenceCounted
/** Adds one to the usage count of this class */
Subclass * Retain()
{
if (mRefCount == 0)
{
abort();
}
if (mRefCount == std::numeric_limits<count_type>::max())
{
abort();
Expand Down
2 changes: 1 addition & 1 deletion src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ static void HandleNodeIdResolve(void * context, DnssdService * result, CHIP_ERRO
static void HandleNodeBrowse(void * context, DnssdService * services, size_t servicesSize, CHIP_ERROR error)
{
ResolverDelegateProxy * proxy = static_cast<ResolverDelegateProxy *>(context);
proxy->Release();

for (size_t i = 0; i < servicesSize; ++i)
{
Expand All @@ -144,6 +143,7 @@ static void HandleNodeBrowse(void * context, DnssdService * services, size_t ser
HandleNodeResolve(context, &services[i], error);
}
}
proxy->Release();
}

CHIP_ERROR AddPtrRecord(DiscoveryFilter filter, const char ** entries, size_t & entriesCount, char * buffer, size_t bufferLen)
Expand Down
2 changes: 1 addition & 1 deletion third_party/openthread/ot-qorvo
Submodule ot-qorvo updated 47 files
+2 −2 .github/workflows/build.yml
+0 −49 .gitignore
+8 −10 CMakeLists.txt
+1 −2 README.md
+0 −36 etc/options.cmake
+1 −1 openthread
+6 −7 script/build
+1 −1 script/test
+8 −72 src/gp712/CMakeLists.txt
+0 −44 src/gp712/crypto/gp712-mbedtls-config.h
+0 −385 src/gp712/uart-socket.c
+7 −57 src/qpg6095/CMakeLists.txt
+3 −13 src/qpg6095/crypto/qpg6095-mbedtls-config.h
+7 −57 src/qpg6100/CMakeLists.txt
+3 −13 src/qpg6100/crypto/qpg6100-mbedtls-config.h
+3 −0 src/qpg6100/platform.c
+0 −96 src/qpg6105/CMakeLists.txt
+0 −62 src/qpg6105/alarm.c
+0 −78 src/qpg6105/alarm_qorvo.h
+0 −42 src/qpg6105/arm-none-eabi.cmake
+0 −230 src/qpg6105/crypto/aes_alt.h
+0 −163 src/qpg6105/crypto/ccm_alt.h
+0 −233 src/qpg6105/crypto/ecjpake_alt.h
+0 −657 src/qpg6105/crypto/ecp_alt.h
+0 −44 src/qpg6105/crypto/qpg6105-mbedtls-config.h
+0 −194 src/qpg6105/crypto/sha256_alt.h
+0 −78 src/qpg6105/diag.c
+0 −49 src/qpg6105/entropy.c
+0 −50 src/qpg6105/logging.c
+0 −49 src/qpg6105/misc.c
+0 −43 src/qpg6105/misc_qorvo.h
+0 −40 src/qpg6105/openthread-core-qpg6105-config-check.h
+0 −53 src/qpg6105/openthread-core-qpg6105-config.h
+0 −89 src/qpg6105/platform.c
+0 −59 src/qpg6105/platform_qorvo.h
+0 −420 src/qpg6105/radio.c
+0 −220 src/qpg6105/radio_qorvo.h
+0 −56 src/qpg6105/random_qorvo.h
+0 −129 src/qpg6105/settings.cpp
+0 −59 src/qpg6105/settings_qorvo.h
+0 −90 src/qpg6105/uart.c
+0 −89 src/qpg6105/uart_qorvo.h
+8 −72 src/qpg7015m/CMakeLists.txt
+0 −44 src/qpg7015m/crypto/qpg7015m-mbedtls-config.h
+34 −0 src/qpg7015m/logging.c
+0 −385 src/qpg7015m/uart-socket.c
+8 −20 third_party/Qorvo/CMakeLists.txt
2 changes: 1 addition & 1 deletion third_party/pigweed/repo
Submodule repo updated 95 files
+0 −1 docs/BUILD.gn
+0 −6 docs/_static/css/pigweed.css
+0 −10 docs/conf.py
+0 −1 docs/index.rst
+0 −637 docs/size_optimizations.rst
+30 −30 pw_allocator/docs.rst
+6 −1 pw_assert/assert_backend_compile_test.cc
+5 −1 pw_assert/assert_backend_compile_test_c.c
+0 −8 pw_assert/docs.rst
+7 −0 pw_assert/public/pw_assert/check.h
+0 −7 pw_assert/public/pw_assert/config.h
+32 −66 pw_assert/public/pw_assert/internal/check_impl.h
+17 −489 pw_chrono/docs.rst
+0 −2 pw_console/py/log_store_test.py
+7 −8 pw_console/py/log_view_test.py
+8 −9 pw_console/py/pw_console/__main__.py
+22 −110 pw_console/py/pw_console/console_app.py
+2 −27 pw_console/py/pw_console/console_prefs.py
+26 −14 pw_console/py/pw_console/embed.py
+47 −0 pw_console/py/pw_console/key_bindings.py
+0 −5 pw_console/py/pw_console/log_line.py
+2 −3 pw_console/py/pw_console/log_view.py
+1 −52 pw_console/py/pw_console/python_logging.py
+1 −4 pw_console/py/pw_console/repl_pane.py
+4 −19 pw_console/py/pw_console/style.py
+20 −22 pw_console/py/pw_console/widgets/table.py
+3 −4 pw_console/py/pw_console/widgets/window_pane.py
+5 −48 pw_console/py/pw_console/widgets/window_pane_toolbar.py
+28 −236 pw_console/py/pw_console/window_list.py
+16 −78 pw_console/py/pw_console/window_manager.py
+1 −1 pw_console/py/setup.cfg
+3 −8 pw_console/py/table_test.py
+29 −123 pw_console/py/window_manager_test.py
+0 −31 pw_console/testing.rst
+4 −4 pw_env_setup/py/pw_env_setup/cipd_setup/pigweed.json
+7 −7 pw_env_setup/py/pw_env_setup/virtualenv_setup/constraint.list
+1 −1 pw_env_setup/pypi_common_setup.cfg
+0 −2 pw_env_setup/util.sh
+3 −5 pw_file/flat_file_system.cc
+1 −1 pw_file/public/pw_file/flat_file_system.h
+22 −14 pw_log/public/pw_log/log.h
+1 −1 pw_log_rpc/public/pw_log_rpc/log_service.h
+6 −6 pw_log_rpc/rpc_log_drain_test.cc
+1 −2 pw_metric/public/pw_metric/metric_service_nanopb.h
+3 −4 pw_presubmit/py/pw_presubmit/build.py
+2 −2 pw_presubmit/py/pw_presubmit/cpp_checks.py
+4 −8 pw_presubmit/py/pw_presubmit/format_code.py
+1 −6 pw_presubmit/py/pw_presubmit/git_repo.py
+13 −6 pw_presubmit/py/pw_presubmit/presubmit.py
+0 −14 pw_protobuf/message.cc
+0 −17 pw_protobuf/message_test.cc
+0 −19 pw_protobuf/public/pw_protobuf/message.h
+1 −1 pw_rpc/client_integration_test.cc
+3 −3 pw_rpc/docs.rst
+1 −1 pw_rpc/nanopb/client_integration_test.cc
+2 −3 pw_rpc/nanopb/codegen_test.cc
+3 −7 pw_rpc/nanopb/docs.rst
+2 −4 pw_rpc/nanopb/method_lookup_test.cc
+1 −2 pw_rpc/nanopb/public/pw_rpc/echo_service_nanopb.h
+1 −0 pw_rpc/nanopb/public/pw_rpc/nanopb/server_reader_writer.h
+1 −1 pw_rpc/nanopb/server_reader_writer_test.cc
+1 −2 pw_rpc/public/pw_rpc/benchmark.h
+12 −2 pw_rpc/public/pw_rpc/channel.h
+6 −1 pw_rpc/public/pw_rpc/client.h
+7 −0 pw_rpc/public/pw_rpc/internal/call.h
+5 −0 pw_rpc/public/pw_rpc/internal/channel.h
+64 −28 pw_rpc/py/pw_rpc/codegen.py
+33 −1 pw_rpc/py/pw_rpc/codegen_nanopb.py
+1 −1 pw_rpc/py/pw_rpc/codegen_raw.py
+1 −2 pw_rpc/raw/codegen_test.cc
+1 −1 pw_rpc/raw/method_info_test.cc
+2 −0 pw_rpc/raw/public/pw_rpc/raw/server_reader_writer.h
+1 −1 pw_rpc/raw/server_reader_writer_test.cc
+31 −40 pw_software_update/bundled_update_service.cc
+1 −6 pw_software_update/public/pw_software_update/bundled_update_service.h
+16 −3 pw_software_update/py/dev_sign_test.py
+10 −1 pw_software_update/py/pw_software_update/dev_sign.py
+6 −6 pw_stream/public/pw_stream/std_file_stream.h
+0 −1 pw_stream/std_file_stream.cc
+1 −1 pw_string/public/pw_string/string_builder.h
+3 −1 pw_string/string_builder.cc
+2 −14 pw_string/string_builder_test.cc
+1 −1 pw_thread_embos/public/pw_thread_embos/id_inline.h
+1 −1 pw_thread_embos/public/pw_thread_embos/yield_inline.h
+0 −1 pw_thread_freertos/BUILD.gn
+1 −1 pw_thread_freertos/thread.cc
+1 −2 pw_trace_tokenized/CMakeLists.txt
+1 −2 pw_trace_tokenized/public/pw_trace_tokenized/trace_rpc_service_nanopb.h
+23 −30 pw_transfer/context.cc
+1 −5 pw_transfer/docs.rst
+11 −1 pw_transfer/public/pw_transfer/internal/context.h
+1 −1 pw_transfer/public/pw_transfer/transfer.h
+1 −2 pw_transfer/test_rpc_server.cc
+5 −27 pw_transfer/transfer_test.cc
+1 −2 pw_unit_test/public/pw_unit_test/unit_test_service.h
2 changes: 1 addition & 1 deletion third_party/qpg_sdk/repo
Submodule repo updated from edb134 to 874261

0 comments on commit 2e44597

Please sign in to comment.