ARTEMIS-3593: Be defensive when reading data from ActiveMQBuffer and allocating memory.

artemis-core-client/src/main/java/org/apache/activemq/artemis/utils/XidCodecSupport.java

@@ -32,12 +32,29 @@ public static void encodeXid(final Xid xid, final ActiveMQBuffer out) {
       out.writeBytes(xid.getGlobalTransactionId());
    }
 
+   private static byte[] safeReadBytes(final ActiveMQBuffer in) {
+      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 > readableBytes) {
+         throw new XidPayloadException("Attempted to read: " + claimedSize +
+                 " which exceeds overall readable buffer size of: " + readableBytes);
+      }
+      final byte[] byteBuffer = new byte[claimedSize];
+      in.readBytes(byteBuffer);
+      return byteBuffer;
+   }
+
    public static Xid decodeXid(final ActiveMQBuffer in) {
       int formatID = in.readInt();
-      byte[] bq = new byte[in.readInt()];
-      in.readBytes(bq);
-      byte[] gtxid = new byte[in.readInt()];
-      in.readBytes(gtxid);
+      byte[] bq = safeReadBytes(in);
+      byte[] gtxid = safeReadBytes(in);
       return new XidImpl(bq, formatID, gtxid);
    }
 

artemis-core-client/src/main/java/org/apache/activemq/artemis/utils/XidPayloadException.java

@@ -0,0 +1,12 @@
+package org.apache.activemq.artemis.utils;
+
+/**
+ * Special type of exception with stacktrace removed which is thrown when binary payload representing
+ * [{@link javax.transaction.xa.Xid}] is invalid.
+ */
+public class XidPayloadException extends RuntimeException {
+
+    public XidPayloadException(String message) {
+        super(message, null, true, false);
+    }
+}

artemis-core-client/src/test/java/org/apache/activemq/artemis/util/XidCodecSupportTest.java

@@ -0,0 +1,52 @@
+package org.apache.activemq.artemis.util;
+
+import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
+import org.apache.activemq.artemis.api.core.ActiveMQBuffers;
+import org.apache.activemq.artemis.core.transaction.impl.XidImpl;
+import org.apache.activemq.artemis.utils.UUIDGenerator;
+import org.apache.activemq.artemis.utils.XidCodecSupport;
+import org.apache.activemq.artemis.utils.XidPayloadException;
+import org.junit.Test;
+
+import javax.transaction.xa.Xid;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+public class XidCodecSupportTest {
+
+    private static final Xid VALID_XID =
+            new XidImpl("xa1".getBytes(), 1, UUIDGenerator.getInstance().generateStringUUID().getBytes());
+
+    @Test
+    public void testEncodeDecode() {
+        final ActiveMQBuffer buffer = ActiveMQBuffers.dynamicBuffer(0);
+        XidCodecSupport.encodeXid(VALID_XID, buffer);
+
+        assertThat(buffer.readableBytes(), equalTo(51)); // formatId(4) + branchQualLength(4) + branchQual(3) +
+        // globalTxIdLength(4) + globalTx(36)
+
+        final Xid readXid = XidCodecSupport.decodeXid(buffer);
+        assertThat(readXid, equalTo(VALID_XID));
+    }
+
+    @Test(expected = XidPayloadException.class)
+    public void testNegativeLength() {
+        final ActiveMQBuffer buffer = ActiveMQBuffers.dynamicBuffer(0);
+        XidCodecSupport.encodeXid(VALID_XID, buffer);
+        // Alter branchQualifierLength to be negative
+        buffer.setByte(4, (byte) 0xFF);
+
+        XidCodecSupport.decodeXid(buffer);
+    }
+
+    @Test(expected = XidPayloadException.class)
+    public void testOverflowLength() {
+        final ActiveMQBuffer buffer = ActiveMQBuffers.dynamicBuffer(0);
+        XidCodecSupport.encodeXid(VALID_XID, buffer);
+        // Alter globalTxIdLength to be too big
+        buffer.setByte(11, (byte) 0x0C);
+
+        XidCodecSupport.decodeXid(buffer);
+    }
+}