CopernicaMarketingSoftware/AMQP-CPP

`AMQP::Reliable` might invoke use-after-free

Closed this issue · 2 comments

Describe the bug
AMQP::Tagger which is a default base class for AMQP::Reliable sets up a channel-wide onError callback in constructor, but doesn't clean it up in destructor, which leads to use-after-free under some circumstances.

Expected behavior and actual behavior
Expected behavior: no use-after-free with straightforward initialization/destruction order.
Actual behavior: use-after-free if AMQP::Reliable is default-destroyed before wrapped AMQP::Channel, and AMQP::Channel somehow invokes its onError callbacks.

Sample code
Provide a small piece of C++ code to demonstrate the issue. Watch out: do not use threads! AMQP-CPP objects are not thread-safe by design. If you construct channels or call methods from one thread, while the event loop is running in a different thread, unexpected things can and will happen.

General idea:

Lets setup AMQP::ConnectionHandler in such a way that it will call 
AMQP::Connection::fail() from `onData`, when called from `ChannelImpl` destructor.

Then this straightforward creation/destruction order leads to use-after-free, 
when AMQP::Connection::fail() in turn calls `reportError()` on channel, 
because there is a reference to already destroyed `reliable` in error callback

{
    AMQP::Channel channel(connection);
    AMQP::Reliable reliable(channel);
}

You are right, this is indeed incorrect. It must however be said that the normal behavior is that when you use a AMQP::Reliable object, that you normally do that for the lifetime of the channel. If you construct an AMQP::Reliable and then destruct it but you keep on using the same channel, you might run into other issues as well, because the channel is still in 'confirm-select' mode.

Many thanks for the fix!