[jira] [Work logged] (ARTEMIS-3593) OOM error on rogue message to Artemis Broker

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/ARTEMIS-3593?focusedWorklogId=687919&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-687919 ]

ASF GitHub Bot logged work on ARTEMIS-3593:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 30/Nov/21 10:21
            Start Date: 30/Nov/21 10:21
    Worklog Time Spent: 10m 
      Work Description: vkolomeyko commented on a change in pull request #3862:
URL: https://github.com/apache/activemq-artemis/pull/3862#discussion_r759132405



##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/utils/XidCodecSupport.java
##########
@@ -33,15 +33,20 @@ public static void encodeXid(final Xid xid, final ActiveMQBuffer out) {
    }
 
    private static byte[] safeReadBytes(final ActiveMQBuffer in) {
-      int claimedSize = in.readInt();
-      int bufferCapacity = in.capacity();
+      final int claimedSize = in.readInt();
+
+      if (claimedSize < 0) {
+         throw new XidPayloadException("Payload size cannot be negative");
+      }
+
+      final int readableBytes = in.readableBytes();
       // We have to be defensive here and not try to allocate byte buffer straight from information available in the
       // stream. Or else, an adversary may handcraft the packet causing OOM situation for a running JVM.
-      if (claimedSize > bufferCapacity) {
-         throw new IllegalStateException("Buffer size: " + claimedSize +
-                 " exceeds overall buffer size of: " + bufferCapacity);
+      if (claimedSize > readableBytes) {
+         throw new XidPayloadException("Attempted to read: " + claimedSize +

Review comment:
       This was to address input from @franz1981 : https://github.com/apache/activemq-artemis/pull/3862#issuecomment-974047110
   To create a custom exception that does not have stacktrace populated.
   
   @clebertsuconic - I note your remark on `ActiveMQAMQPIllegalStateException` and I am flexible to use either approach. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Issue Time Tracking
-------------------

            Worklog Id:     (was: 687919)
    Remaining Estimate: 0h
            Time Spent: 10m

> OOM error on rogue message to Artemis Broker
> --------------------------------------------
>
>                 Key: ARTEMIS-3593
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-3593
>             Project: ActiveMQ Artemis
>          Issue Type: Bug
>          Components: Broker
>    Affects Versions: 2.6.2
>            Reporter: Viktor Kolomeyko
>            Priority: Critical
>             Fix For: 2.20.0
>
>         Attachments: CrashDump.log, dospayload.binary
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> A problem been reported by a Security Researcher when a Java process running an embedded Artemis Broker been sent a handcrafted message:
> {code:sh}
> cat /path/to/dospayload.binary > /dev/tcp/<broker_address>/<broker_port>{code}
> resulting OutOfMemory crash, please see attachment.
> The problem is caused by the fact that a 32-bit integer is read from the stream and byte array is allocated using this value without performing any checks.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)