fix(cluster): reject truncated command payloads with MALFORMED_COMMAND

lib/BoundCluster.js

@@ -4,6 +4,7 @@ let { debug } = require('./util');
 const { ZCLDataType } = require('./zclTypes');
 const { getLogId, getPropertyDescriptor } = require('./util');
 const Cluster = require('./Cluster');
+const parseCommandArgs = require('./util/parseCommandArgs');
 
 debug = debug.extend('bound-cluster');
 
@@ -308,8 +309,9 @@ class BoundCluster {
       .pop();
 
     if (command) {
+      // Validate payload length before parsing; see lib/util/parseCommandArgs.js.
       const args = command.args
-        ? command.args.fromBuffer(frame.data, 0)
+        ? parseCommandArgs(command, frame.data)
         : undefined;
 
       if (this[command.name]) {

lib/Cluster.js

@@ -4,6 +4,7 @@ const EventEmitter = require('events');
 
 let { debug } = require('./util');
 const { getLogId } = require('./util');
+const parseCommandArgs = require('./util/parseCommandArgs');
 
 debug = debug.extend('cluster');
 
@@ -775,9 +776,11 @@ class Cluster extends EventEmitter {
     if (command) {
       const handlerName = `on${command.name.charAt(0).toUpperCase()}${command.name.slice(1)}`;
 
-      // Parse the command arguments
+      // Parse the command arguments. Validate the payload length first so that
+      // truncated frames are rejected instead of silently parsed with zero-filled
+      // fields (see lib/util/parseArgs.js for details).
       const args = command.args
-        ? command.args.fromBuffer(frame.data, 0)
+        ? parseCommandArgs(command, frame.data)
         : undefined;
 
       debug(this.logId, 'received frame', command.name, args);

lib/util/parseCommandArgs.js

@@ -0,0 +1,56 @@
+'use strict';
+
+const { ZCLError } = require('./index');
+
+/**
+ * Parse a ZCL command's argument payload with defensive length validation.
+ *
+ * The underlying parser primitives in `@athombv/data-types` silently fall back
+ * to zero-filled values when the source buffer is shorter than the declared
+ * field length. For example, an empty payload parsed against a fixed-size
+ * struct produces an object where every numeric field is 0 and every bitmap
+ * field has all bits clear. This is unsafe for alarm-routing clusters (IAS
+ * Zone) and for status-bearing response frames, where the zero values happen
+ * to mean "all clear" / "SUCCESS".
+ *
+ * This helper rejects truncated payloads up-front by throwing a ZCLError with
+ * status `MALFORMED_COMMAND`, which `Endpoint.handleFrame` converts into a
+ * proper default-response frame back to the sender.
+ *
+ * @param {object} command - Cluster command definition (with `args` Struct).
+ * @param {Buffer} data - Frame payload bytes.
+ * @returns {object} Parsed args instance.
+ * @throws {ZCLError} With zclStatus `MALFORMED_COMMAND` when parsing fails or
+ *   the payload is shorter than the args struct's minimum byte length.
+ */
+function parseCommandArgs(command, data) {
+  const argsType = command.args;
+
+  // Commands declared with `encodeMissingFieldsBehavior: 'skip'` allow trailing
+  // fields to be omitted (e.g. OTA `hardwareVersion` is only present when its
+  // fieldControl bit is set). For those we cannot derive a strict minimum from
+  // the struct definition and rely on the try/catch below to catch overruns.
+  if (command.encodeMissingFieldsBehavior !== 'skip') {
+    // Struct.length convention:
+    //   > 0 : exact byte count for an all-fixed-size struct
+    //   < 0 : negative of the fixed-portion size for a varsize struct
+    //   = 0 : no payload required
+    // For both fixed and varsize structs, |declaredLength| is the minimum
+    // number of bytes the payload must carry to populate the fixed fields.
+    const minLength = Math.abs(argsType.length);
+    if (data.length < minLength) {
+      throw new ZCLError('MALFORMED_COMMAND');
+    }
+  }
+
+  try {
+    return argsType.fromBuffer(data, 0);
+  } catch (err) {
+    // The parser threw mid-walk (e.g. a varsize length prefix overran the
+    // payload). Treat as malformed rather than letting the raw RangeError
+    // propagate as a generic FAILURE.
+    throw new ZCLError('MALFORMED_COMMAND');
+  }
+}
+
+module.exports = parseCommandArgs;

test/testMalformedFrames.js

@@ -0,0 +1,163 @@
+'use strict';
+
+const assert = require('assert');
+const Node = require('../lib/Node');
+const BoundCluster = require('../lib/BoundCluster');
+const IASZoneCluster = require('../lib/clusters/iasZone');
+const { ZCLStandardHeader } = require('../lib/zclFrames');
+const { ZCLDataTypes } = require('../lib/zclTypes');
+
+/**
+ * Helper: build a mock Node that captures every frame sent back via sendFrame.
+ * Unlike `createMockNode({loopback:true})`, this lets the test inspect the
+ * default-response frames that handleFrame produces (e.g. MALFORMED_COMMAND).
+ */
+function createCapturingNode(endpointId, inputClusterId) {
+  const sentFrames = [];
+  const mockNode = {
+    sendFrame: async (epId, clId, data) => {
+      sentFrames.push({
+        endpointId: epId, clusterId: clId, raw: data,
+      });
+    },
+    endpointDescriptors: [{
+      endpointId,
+      inputClusters: [inputClusterId],
+      outputClusters: [],
+    }],
+  };
+  const node = new Node(mockNode);
+  return { node, sentFrames };
+}
+
+/** Parse a captured ZCL default-response frame back to {cmdId, status}. */
+function parseDefaultResponse(rawFrame) {
+  const frame = ZCLStandardHeader.fromBuffer(rawFrame);
+  // ZCL default response cmdId is 0x0B
+  if (frame.cmdId !== 0x0B) return null;
+  return {
+    forCmdId: frame.data.readUInt8(0),
+    status: ZCLDataTypes.enum8Status.fromBuffer(frame.data, 1),
+  };
+}
+
+describe('Malformed frame hardening', function() {
+  describe('Cluster.handleFrame (server-to-client direction)', function() {
+    it('should not invoke handler on truncated zoneStatusChangeNotification', function(done) {
+      const { node } = createCapturingNode(1, IASZoneCluster.ID);
+
+      node.endpoints[1].clusters.iasZone.onZoneStatusChangeNotification = data => {
+        done(new Error(`handler invoked on malformed frame: ${JSON.stringify(data)}`));
+      };
+
+      const frame = new ZCLStandardHeader();
+      frame.cmdId = IASZoneCluster.COMMANDS.zoneStatusChangeNotification.id;
+      frame.frameControl.directionToClient = true;
+      frame.frameControl.clusterSpecific = true;
+      // Empty payload: the fixed-size args (6 bytes) cannot be parsed.
+      // Pre-fix behavior: silently parses as zoneStatus.alarm1=false, extendedStatus=0,
+      // zoneId=0, delay=0 - a fail-open read for alarm-routing drivers.
+      frame.data = Buffer.alloc(0);
+
+      node.handleFrame(1, IASZoneCluster.ID, frame.toBuffer(), {})
+        .then(() => done())
+        .catch(done);
+    });
+
+    it('should respond with MALFORMED_COMMAND on truncated zoneStatusChangeNotification', async function() {
+      const { node, sentFrames } = createCapturingNode(1, IASZoneCluster.ID);
+
+      const frame = new ZCLStandardHeader();
+      frame.cmdId = IASZoneCluster.COMMANDS.zoneStatusChangeNotification.id;
+      frame.frameControl.directionToClient = true;
+      frame.frameControl.clusterSpecific = true;
+      frame.data = Buffer.from([0x10, 0x00]); // 2 bytes, needs 6
+
+      await node.handleFrame(1, IASZoneCluster.ID, frame.toBuffer(), {});
+
+      // Find the default-response frame the receiver sent back.
+      const response = sentFrames
+        .map(f => parseDefaultResponse(f.raw))
+        .find(r => r && r.forCmdId === IASZoneCluster.COMMANDS.zoneStatusChangeNotification.id);
+
+      assert.ok(response, 'expected a default-response frame to be sent back');
+      assert.strictEqual(response.status, 'MALFORMED_COMMAND',
+        `expected MALFORMED_COMMAND, got ${response.status}`);
+    });
+
+    it('should still invoke handler on well-formed zoneStatusChangeNotification (regression)', function(done) {
+      const { node } = createCapturingNode(1, IASZoneCluster.ID);
+
+      node.endpoints[1].clusters.iasZone.onZoneStatusChangeNotification = data => {
+        try {
+          assert.strictEqual(data.zoneStatus.alarm1, true);
+          assert.strictEqual(data.extendedStatus, 0);
+          assert.strictEqual(data.zoneId, 10);
+          assert.strictEqual(data.delay, 108);
+          done();
+        } catch (err) {
+          done(err);
+        }
+      };
+
+      const frame = new ZCLStandardHeader();
+      frame.cmdId = IASZoneCluster.COMMANDS.zoneStatusChangeNotification.id;
+      frame.frameControl.directionToClient = true;
+      frame.frameControl.clusterSpecific = true;
+      // alarm1 bit set in zoneStatus (map16=0x0001), extendedStatus=0, zoneId=10, delay=108
+      frame.data = Buffer.from([0x01, 0x00, 0x00, 0x0A, 0x6C, 0x00]);
+
+      node.handleFrame(1, IASZoneCluster.ID, frame.toBuffer(), {}).catch(done);
+    });
+  });
+
+  describe('BoundCluster.handleFrame (client-to-server direction)', function() {
+    it('should not invoke handler on truncated zoneEnrollResponse', async function() {
+      const { node } = createCapturingNode(1, IASZoneCluster.ID);
+
+      let handlerCalled = false;
+      node.endpoints[1].bind('iasZone', new (class extends BoundCluster {
+
+        async zoneEnrollResponse(data) {
+          handlerCalled = true;
+          throw new Error(`handler invoked on malformed frame: ${JSON.stringify(data)}`);
+        }
+
+      })());
+
+      const frame = new ZCLStandardHeader();
+      frame.cmdId = IASZoneCluster.COMMANDS.zoneEnrollResponse.id;
+      frame.frameControl.directionToClient = false;
+      frame.frameControl.clusterSpecific = true;
+      // Empty payload: enrollResponseCode (1 byte) and zoneId (1 byte) cannot be parsed.
+      // Pre-fix behavior: silently parses as enrollResponseCode='success', zoneId=0.
+      frame.data = Buffer.alloc(0);
+
+      await node.handleFrame(1, IASZoneCluster.ID, frame.toBuffer(), {});
+
+      assert.strictEqual(handlerCalled, false, 'handler must not be called on truncated frame');
+    });
+
+    it('should respond with MALFORMED_COMMAND on truncated zoneEnrollResponse', async function() {
+      const { node, sentFrames } = createCapturingNode(1, IASZoneCluster.ID);
+
+      node.endpoints[1].bind('iasZone', new BoundCluster());
+
+      const frame = new ZCLStandardHeader();
+      frame.cmdId = IASZoneCluster.COMMANDS.zoneEnrollResponse.id;
+      frame.frameControl.directionToClient = false;
+      frame.frameControl.clusterSpecific = true;
+      frame.data = Buffer.from([0x00]); // 1 byte, needs 2
+
+      await node.handleFrame(1, IASZoneCluster.ID, frame.toBuffer(), {});
+
+      const response = sentFrames
+        .map(f => parseDefaultResponse(f.raw))
+        .find(r => r && r.forCmdId === IASZoneCluster.COMMANDS.zoneEnrollResponse.id);
+
+      assert.ok(response, 'expected a default-response frame to be sent back');
+      assert.strictEqual(response.status, 'MALFORMED_COMMAND',
+        `expected MALFORMED_COMMAND, got ${response.status}`);
+    });
+  });
+});