From adf4fb3bc17a51eccf75d756f6c89f0d0a46f3f4 Mon Sep 17 00:00:00 2001 From: "Peter A. Bigot" Date: Tue, 24 Apr 2018 14:50:31 -0500 Subject: [PATCH 1/2] TcpResolver: reduce risk of accessing destructed TcpConnection Invoking TcpHandler::onError might result in the connection being destroyed. Though the reference to it in TcpClosed() is likely benign, it's safer to follow the standard practice of returning a nullptr to indicate that the connection is known to be destroyed. --- src/linux_tcp/tcpresolver.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/linux_tcp/tcpresolver.h b/src/linux_tcp/tcpresolver.h index 5fc002a..abca375 100644 --- a/src/linux_tcp/tcpresolver.h +++ b/src/linux_tcp/tcpresolver.h @@ -194,7 +194,7 @@ public: * Proceed to the next state * @return TcpState * */ - TcpState *proceed() + TcpState *proceed(const Monitor &monitor) { // do we have a valid socket? if (_socket >= 0) @@ -211,6 +211,9 @@ public: // report error _handler->onError(_connection, _error.data()); + // handler callback might have destroyed connection + if (!monitor.valid()) return nullptr; + // create dummy implementation return new TcpClosed(_connection, _handler); } @@ -229,7 +232,7 @@ public: if (fd != _pipe.in() || !(flags & readable)) return this; // proceed to the next state - return proceed(); + return proceed(monitor); } /** @@ -243,7 +246,7 @@ public: _thread.join(); // proceed to the next state - return proceed(); + return proceed(monitor); } /** @@ -262,4 +265,3 @@ public: * End of namespace */ } - From 94bff62986e6489727d1dc57cd9fb68ba62de596 Mon Sep 17 00:00:00 2001 From: "Peter A. Bigot" Date: Thu, 26 Apr 2018 06:06:41 -0500 Subject: [PATCH 2/2] Monitor: avoid null pointer dereference when copying instances When the copy constructor was added to allow passing a monitor by value into a lambda the implementation did not account for the possibility of the watchable having already been destroyed. Also provide the companion copy assignment to complete the triad. --- include/amqpcpp/monitor.h | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/include/amqpcpp/monitor.h b/include/amqpcpp/monitor.h index efb788c..321b76e 100644 --- a/include/amqpcpp/monitor.h +++ b/include/amqpcpp/monitor.h @@ -64,7 +64,25 @@ public: Monitor(const Monitor &monitor) : _watchable(monitor._watchable) { // register with the watchable - _watchable->add(this); + if (_watchable) _watchable->add(this); + } + + /** + * Assignment operator + * @param monitor + */ + Monitor& operator= (const Monitor &monitor) + { + // remove from watchable + if (_watchable) _watchable->remove(this); + + // replace watchable + _watchable = monitor._watchable; + + // register with the watchable + if (_watchable) _watchable->add(this); + + return *this; } /**