[
https://issues.apache.org/jira/browse/QPID-7379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15522944#comment-15522944
]
Rob Godfrey commented on QPID-7379:
-----------------------------------
{quote}
* AbstractVirtualHost#importMessageStore can be used for a DoS attack by
crafting a "store" containing just 5 bytes: "0x00 MAX_INT" which will allocate
a byte array of 2 GB which potentially exhaust the broker's heap bringing down
the broker with an OOM Error. Maybe we limit the version string length to 1
byte? In that case the arbitrary 50 in data.mark(50) could be replaced with an
accurate upper bound on the reads like 1+1+256. I guess a similar DoS can be
performed at many point within the serialisation stream. I think we should
guard against this by disallowing deserialisation of messages that exceed
qpid.max_message_size.
{quote}
I think anyone who has sufficient privileges to import a data store probably
also has all sorts of other ways to bring down the broker. I think the issue
with qpid.max_message_size is that we don't know that the source host had the
same or smaller limit. Overall I don't see this as a priority.
{quote}
* I believe the 0 that is expected at the beginning of
AbstractVirtualHost#importMessageStore is actually a
serializer.v1.RecordType#VERSION
{quote}
No - the text in the comment is an accurate reflection of how I believe it
should be interpreted... all future versions must also begin "0" <int size>
<size * octets of utf-8 version string>. For the v1 format this also looks
like a standard record, but that can be considered coincidence.
{quote}
* It might be nicer to just throw the data stream at all serializers that are
available through the QpidServiceLoader and have them handle or reject the data
instead of putting knowledge of the serialisation format into the
AbstractVirtualHost
{quote}
I think the "correct" change would be to extract this logic into some sort of
MessageStoreSerializerFactory or something - but I'm against just throwing the
data stream at the serializers. Really in order to be able to reset to the
start the information necessary to determine the format needs to be in some
sort of header at the start.
{quote}
* Why do you use arrays instead of direct memory? This seems especially odd for
potentially large chunks like the message content. Also, is it necessary to
make an private copy the data in the MessageRecord? Could you not just copy it
from the storedMessage in MessageRecord.getData()?
{quote}
OutputStreams only work with byte[], not ByteBuffers which means heap memory
has to be used. I don't believe (given the prior constraint) that any
unnecessary copy is made... on reading the record we read the bytes into an
array from the output stream. On writing we create an array and copy the data
from the content into the array. This value is then written to the
outputstream.
All the other points seem valid and I agree should be fixed
> [Java Broker] Provide a mechanism to extract messages from a vhost message
> store and replay them into a new vhost
> -----------------------------------------------------------------------------------------------------------------
>
> Key: QPID-7379
> URL: https://issues.apache.org/jira/browse/QPID-7379
> Project: Qpid
> Issue Type: Improvement
> Components: Java Broker
> Reporter: Rob Godfrey
> Assignee: Rob Godfrey
> Fix For: qpid-java-6.1
>
>
> QPID-7359 provided operations to extract the config from a virtual host, but
> there are not currently any mechanisms to extract the contents of the message
> store and replay that into a new vhost. We should add this feature to (for
> example) allow people to migrate their data from one vhost type to another
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
