Bug 693407 - Invalid client code will crash the broker daemon.
Summary: Invalid client code will crash the broker daemon.
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise MRG
Classification: Red Hat
Component: qpid-cpp
Version: 1.3
Hardware: i686
OS: Linux
urgent
urgent
Target Milestone: 2.0
: ---
Assignee: Gordon Sim
QA Contact: Petr Matousek
URL:
Whiteboard:
Depends On:
Blocks: 700506
TreeView+ depends on / blocked
 
Reported: 2011-04-04 14:59 UTC by Nick Capito
Modified: 2011-06-23 15:44 UTC (History)
4 users (show)

Fixed In Version: qpid-cpp-mrg-0.10-4
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 700506 (view as bug list)
Environment:
Last Closed: 2011-06-23 15:44:19 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHEA-2011:0890 0 normal SHIPPED_LIVE Red Hat Enterprise MRG Messaging 2.0 Release 2011-06-23 15:42:41 UTC

Description Nick Capito 2011-04-04 14:59:27 UTC
Incorrect code will crash broker.  By accident we have discovered that the following code will cause the broker to crash.


We know that this is invalid syntax, but feel that it should not result in a a broker crash.


def test_bad_client_code():
    debug = True
    
    name = "test"
    name +=  '''; { 
      create: always | sender | receiver | never,
      delete: always | sender | receiver | never,
      assert: always | sender | receiver | never,
      mode: consume, \
      node: { \
        type: topic ,\
        durable: True ,\
        x-declare: { x-qpid.trace='%s'} \
      }''' % "test"
    session = Connection.establish("localhost:5672").session(name=name)
    sender = session.sender(name)
    sender.send(Message(content="Test", subject="Nick.3G"))
    sender.sync()

Comment 1 Gordon Sim 2011-04-04 15:51:26 UTC
Even simpler trigger for the crash is:

from qpid.messaging import *

session = Connection.establish("localhost:5672").session(name="a"*1024)
session.sync()

The root problem is that the Session management object has a name that is defined as a string whose length must fit into a single byte, whereas the AMQP 0-10 protocol defines the session name to be a string whose length is 16 bits.

Thus you can define a session name for which is too large for the object defined in the management schema, and as there is no checking this causes a crash.

Comment 2 Gordon Sim 2011-04-04 16:17:03 UTC
Core was generated by `/home/gordon/projects/qpid-svn-trunk/cpp/build/src/.libs/lt-qpidd --auth no --m'.
Program terminated with signal 6, Aborted.
#0  0x0044a424 in __kernel_vsyscall ()
Missing separate debuginfos, use: debuginfo-install boost-filesystem-1.39.0-9.fc12.i686 boost-program-options-1.39.0-9.fc12.i686 boost-system-1.39.0-9.fc12.i686 cyrus-sasl-lib-2.1.23-9.fc12.i686 glibc-2.11.2-3.i686 libgcc-4.4.4-10.fc12.i686 libstdc++-4.4.4-10.fc12.i686 libuuid-2.16.2-9.fc12.i686 nss-softokn-freebl-3.12.8-1.fc12.i686
(gdb) bt
#0  0x0044a424 in __kernel_vsyscall ()
#1  0x00637b91 in raise () from /lib/libc.so.6
#2  0x0063946a in abort () from /lib/libc.so.6
#3  0x004fe397 in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/libstdc++.so.6
#4  0x004fc226 in ?? () from /usr/lib/libstdc++.so.6
#5  0x004fc263 in std::terminate() () from /usr/lib/libstdc++.so.6
#6  0x004fc3a2 in __cxa_throw () from /usr/lib/libstdc++.so.6
#7  0x002a64d5 in qpid::framing::Buffer::putShortString (this=0xb5701cc8, s="60b548dc-299f-4080-9fe5-aa90876a1956:", 'a' <repeats 1024 times>) at ../../src/qpid/framing/Buffer.cpp:255
#8  0x002c3db6 in qpid::management::Buffer::putShortString (this=0xb77c7e08, i="60b548dc-299f-4080-9fe5-aa90876a1956:", 'a' <repeats 1024 times>) at ../../src/qpid/management/Buffer.cpp:60
#9  0x00e36e78 in qmf::org::apache::qpid::broker::Session::writeProperties (this=0xb5801180, _sBuf="") at qmf/org/apache/qpid/broker/Session.cpp:360
#10 0x00f5b346 in qpid::management::ManagementAgent::DeletedObject::DeletedObject (this=0xb5701bf0, src=0xb5801180, v1=true, v2=true) at ../../src/qpid/management/ManagementAgent.cpp:3013
#11 0x00f63a14 in qpid::management::ManagementAgent::moveNewObjectsLH (this=0xb6da8008) at ../../src/qpid/management/ManagementAgent.cpp:675
#12 0x00f6c917 in qpid::management::ManagementAgent::periodicProcessing (this=0xb6da8008) at ../../src/qpid/management/ManagementAgent.cpp:714
#13 0x00f74c8e in qpid::management::ManagementAgent::Periodic::fire (this=0xb5700468) at ../../src/qpid/management/ManagementAgent.cpp:433
#14 0x002d4655 in qpid::sys::TimerTask::fireTask (this=0xb5700468) at ../../src/qpid/sys/Timer.cpp:57
#15 0x002d4953 in qpid::sys::Timer::fire (this=0x96fc918, t=...) at ../../src/qpid/sys/Timer.cpp:188
#16 0x002d5513 in qpid::sys::Timer::run (this=0x96fc918) at ../../src/qpid/sys/Timer.cpp:123
#17 0x001fdcc1 in qpid::sys::(anonymous namespace)::runRunnable (p=0x96fc918) at ../../src/qpid/sys/posix/Thread.cpp:35
#18 0x007c0925 in start_thread () from /lib/libpthread.so.0
#19 0x006ea07e in clone () from /lib/libc.so.6


The crash can be prevented by some exception handling:

Index: src/qpid/management/ManagementAgent.cpp
===================================================================
--- src/qpid/management/ManagementAgent.cpp	(revision 1088526)
+++ src/qpid/management/ManagementAgent.cpp	(working copy)
@@ -430,7 +430,13 @@
 void ManagementAgent::Periodic::fire ()
 {
     agent.timer->add (new Periodic (agent, agent.interval));
-    agent.periodicProcessing ();
+    try {
+        agent.periodicProcessing ();
+    } catch (const std::exception& e) {
+        QPID_LOG(error, "Exception caught while processing management: " << e.what());
+    } catch (...) {
+        QPID_LOG(error, "Unknown exception caught while processing management");
+    }
 }

a proper solution would be a little more involved in the QMF internals I think...

Comment 3 Gordon Sim 2011-04-04 16:57:26 UTC
More generic safeguard available for review: https://reviews.apache.org/r/546/

Comment 4 Gordon Sim 2011-04-05 10:21:13 UTC
Fix that prevents the crash (though not the underlying issue with management and session names greater than 256 bytes long) committed upstream as http://svn.apache.org/viewvc?view=rev&rev=1088951

Comment 5 Gordon Sim 2011-04-05 12:22:04 UTC
See https://bugzilla.redhat.com/show_bug.cgi?id=693721 for the original QMF issue with session names.

Comment 8 Petr Matousek 2011-04-28 15:06:58 UTC
This issue has been fixed in qpid-cpp-mrg-0.10-4 for RHEL5, but not yet available in any RHEL6 package. 

The bug was cloned for RHEL6: please see BZ700506

Verified on RHEL5.6 architectures: i386, x86_64

packages installed:
python-qpid-0.10-1.el5
python-qpid-qmf-0.10-6.el5
qpid-cpp-client-0.10-4.el5
qpid-cpp-client-devel-0.10-4.el5
qpid-cpp-client-devel-docs-0.10-4.el5
qpid-cpp-client-ssl-0.10-4.el5
qpid-cpp-mrg-debuginfo-0.10-4.el5
qpid-cpp-server-0.10-4.el5
qpid-cpp-server-cluster-0.10-4.el5
qpid-cpp-server-devel-0.10-4.el5
qpid-cpp-server-ssl-0.10-4.el5
qpid-cpp-server-store-0.10-4.el5
qpid-cpp-server-xml-0.10-4.el5
qpid-java-client-0.10-4.el5
qpid-java-common-0.10-4.el5
qpid-java-example-0.10-4.el5
qpid-qmf-0.10-6.el5
qpid-qmf-devel-0.10-6.el5
qpid-tools-0.10-4.el5

-> VERIFIED

Comment 9 errata-xmlrpc 2011-06-23 15:44:19 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHEA-2011-0890.html


Note You need to log in before you can comment on or make changes to this bug.