From abbdc70d8bed7fffc9b32eae7031e6caa0832a42 Mon Sep 17 00:00:00 2001
From: Xiaoyan Lin <linxiaoyan18@gmail.com>
Date: Tue, 15 Mar 2016 22:25:39 -0700
Subject: [PATCH] Validate MQTT CONNECT reserved flag in variable header

Motivation:

According to the MQTT 3.1.1 Protocol Specification: The Server MUST validate that the reserved flag in the CONNECT Control Packet is set to zero and disconnect the Client if it is not zero. (http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc385349230)

Resolves #4182

Modifications:

Check the CONNECT reserved flag for MQTT 3.1.1. If it's not 0, throw an exception.

Result:

If the CONNECT reserved flag, a decode failure will be emitted.
---
 .../netty/handler/codec/mqtt/MqttDecoder.java |  9 ++++++++
 .../handler/codec/mqtt/MqttCodecTest.java     | 23 +++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java b/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java
index 70f0a41e7b..7103bed641 100644
--- a/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java
+++ b/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java
@@ -221,6 +221,15 @@ public class MqttDecoder extends ReplayingDecoder<DecoderState> {
         final int willQos = (b1 & 0x18) >> 3;
         final boolean willFlag = (b1 & 0x04) == 0x04;
         final boolean cleanSession = (b1 & 0x02) == 0x02;
+        if (mqttVersion == MqttVersion.MQTT_3_1_1) {
+            final boolean zeroReservedFlag = (b1 & 0x01) == 0x0;
+            if (!zeroReservedFlag) {
+                // MQTT v3.1.1: The Server MUST validate that the reserved flag in the CONNECT Control Packet is
+                // set to zero and disconnect the Client if it is not zero.
+                // See http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc385349230
+                throw new DecoderException("non-zero reserved flag");
+            }
+        }
 
         final MqttConnectVariableHeader mqttConnectVariableHeader = new MqttConnectVariableHeader(
                 mqttVersion.protocolName(),
diff --git a/codec-mqtt/src/test/java/io/netty/handler/codec/mqtt/MqttCodecTest.java b/codec-mqtt/src/test/java/io/netty/handler/codec/mqtt/MqttCodecTest.java
index 705d071bfd..5068a94139 100644
--- a/codec-mqtt/src/test/java/io/netty/handler/codec/mqtt/MqttCodecTest.java
+++ b/codec-mqtt/src/test/java/io/netty/handler/codec/mqtt/MqttCodecTest.java
@@ -21,6 +21,7 @@ import io.netty.buffer.ByteBufAllocator;
 import io.netty.buffer.UnpooledByteBufAllocator;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.DecoderException;
 import io.netty.util.CharsetUtil;
 import org.easymock.Mock;
 import org.junit.Before;
@@ -96,6 +97,28 @@ public class MqttCodecTest {
         validateConnectPayload(message.payload(), decodedMessage.payload());
     }
 
+    @Test
+    public void testConnectMessageWithNonZeroReservedFlagForMqtt311() throws Exception {
+        final MqttConnectMessage message = createConnectMessage(MqttVersion.MQTT_3_1_1);
+        ByteBuf byteBuf = MqttEncoder.doEncode(ALLOCATOR, message);
+        try {
+            // Set the reserved flag in the CONNECT Packet to 1
+            byteBuf.setByte(9, byteBuf.getByte(9) | 0x1);
+            final List<Object> out = new LinkedList<Object>();
+            mqttDecoder.decode(ctx, byteBuf, out);
+
+            assertEquals("Expected one object bout got " + out.size(), 1, out.size());
+
+            final MqttMessage decodedMessage = (MqttMessage) out.get(0);
+            assertTrue(decodedMessage.decoderResult().isFailure());
+            Throwable cause = decodedMessage.decoderResult().cause();
+            assertTrue(cause instanceof DecoderException);
+            assertEquals("non-zero reserved flag", cause.getMessage());
+        } finally {
+            byteBuf.release();
+        }
+    }
+
     @Test
     public void testConnAckMessage() throws Exception {
         final MqttConnAckMessage message = createConnAckMessage();