From 0cacd49f58eb813205ce5109350a3e9565a059ef Mon Sep 17 00:00:00 2001 From: Robin Bolscher Date: Fri, 23 Oct 2026 10:24:20 +0200 Subject: [PATCH 1/2] fix(cluster): Reject truncated command payloads with MALFORMED_COMMAND The parser primitives in @athombv/data-types silently substitute zero values when the source buffer is shorter than the declared field length. For a ZCL command struct, this means a truncated payload parses into an object where every numeric field is 0 and every bitmap field has all bits clear - which for alarm-routing clusters (IAS Zone) happens to look like "all clear" and for status-bearing response frames happens to look like "SUCCESS". This change adds a length precheck and try/catch around command arg parsing in Cluster.handleFrame and BoundCluster.handleFrame. Truncated payloads now throw ZCLError('MALFORMED_COMMAND'), which Endpoint.handleFrame already converts into a proper default-response back to the sender. Commands declared with encodeMissingFieldsBehavior: 'skip' (OTA) are exempt from the strict length precheck because they intentionally allow trailing fields to be omitted; for those, only the try/catch fallback applies. --- lib/BoundCluster.js | 4 +- lib/Cluster.js | 7 +- lib/util/parseCommandArgs.js | 56 +++++++++++ test/testMalformedFrames.js | 176 +++++++++++++++++++++++++++++++++++ 4 files changed, 240 insertions(+), 3 deletions(-) create mode 100644 lib/util/parseCommandArgs.js create mode 100644 test/testMalformedFrames.js diff --git a/lib/BoundCluster.js b/lib/BoundCluster.js index 63031bd..18908af 100644 --- a/lib/BoundCluster.js +++ b/lib/BoundCluster.js @@ -4,6 +4,7 @@ let { debug } = require('./util'); const { ZCLDataType } = require('./zclTypes'); const { getLogId, getPropertyDescriptor, ZCLError } = require('./util'); const Cluster = require('./Cluster'); +const parseCommandArgs = require('./util/parseCommandArgs'); debug = debug.extend('bound-cluster'); @@ -364,7 +365,8 @@ class BoundCluster { .pop(); if (command) { - const args = command.args ? command.args.fromBuffer(frame.data, 0) : undefined; + // Validate payload length before parsing; see lib/util/parseCommandArgs.js. + const args = command.args ? parseCommandArgs(command, frame.data) : undefined; if (this[command.name]) { debug(this.logId, 'received command', command.name, args); diff --git a/lib/Cluster.js b/lib/Cluster.js index 14277f4..537ccdb 100644 --- a/lib/Cluster.js +++ b/lib/Cluster.js @@ -4,6 +4,7 @@ const EventEmitter = require('node:events'); let { debug } = require('./util'); const { getLogId, ZCLError } = require('./util'); +const parseCommandArgs = require('./util/parseCommandArgs'); debug = debug.extend('cluster'); @@ -862,8 +863,10 @@ class Cluster extends EventEmitter { if (command) { const handlerName = `on${command.name.charAt(0).toUpperCase()}${command.name.slice(1)}`; - // Parse the command arguments - const args = command.args ? command.args.fromBuffer(frame.data, 0) : undefined; + // 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/parseCommandArgs.js for details). + const args = command.args ? parseCommandArgs(command, frame.data) : undefined; debug(this.logId, 'received frame', command.name, args); diff --git a/lib/util/parseCommandArgs.js b/lib/util/parseCommandArgs.js new file mode 100644 index 0000000..647a20c --- /dev/null +++ b/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 { + // 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; diff --git a/test/testMalformedFrames.js b/test/testMalformedFrames.js new file mode 100644 index 0000000..c0fd957 --- /dev/null +++ b/test/testMalformedFrames.js @@ -0,0 +1,176 @@ +'use strict'; + +const { describe, it } = require('node:test'); +const assert = require('node: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 (t, 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 (t, 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}`, + ); + }); + }); +}); From 6d0012821405ca0315e60f90862949d445975d1f Mon Sep 17 00:00:00 2001 From: Robin Bolscher Date: Fri, 23 Oct 2026 10:25:43 +0200 Subject: [PATCH 2/2] fix(cluster): Harden attribute reports and tighten 'skip' exemption Follow-up to the command-arg hardening. Addresses two bypasses that remained after the first pass: 1. Attribute reports (cmdId 0x0A reportAttributes) parse a raw payload into ZCLAttributeDataRecord entries, where each entry's value field was read via `DataType.fromBuffer` without bounds checking. For fixed-width types like map16 (IAS Zone `zoneStatus`, attribute id 0x0002), a missing or short value silently produced an all-bits-false bitmap that propagates to drivers as "all alarms clear". Same fail- open path as the command-arg case from PR #193, just routed through the attribute-report channel. Add a length precheck in `ZCLAttributeDataRecord.fromBuf` before calling the value's `fromBuffer`. On short input, throw `ZCLError('MALFORMED_COMMAND')` which `Endpoint.handleFrame` already converts to a proper default-response back to the sender. 2. The previous `encodeMissingFieldsBehavior: 'skip'` exemption in `parseCommandArgs` was too broad: it skipped *all* length checking for those commands, so an empty payload to e.g. OTA `queryNextImageRequest` parsed as all-zero fields. Tighten to require at least the first field's byte width, which preserves the legitimate "omit trailing optional fields" case while rejecting truncated leading-discriminator payloads. The tests run on the virtual device; iasZone.js and ota.js cover the well-formed frames. --- lib/util/parseCommandArgs.js | 53 ++++++-- lib/zclFrames.js | 11 ++ test/testMalformedFrames.js | 246 ++++++++++++----------------------- 3 files changed, 135 insertions(+), 175 deletions(-) diff --git a/lib/util/parseCommandArgs.js b/lib/util/parseCommandArgs.js index 647a20c..d1b3948 100644 --- a/lib/util/parseCommandArgs.js +++ b/lib/util/parseCommandArgs.js @@ -23,24 +23,49 @@ const { ZCLError } = require('./index'); * @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') { +/** + * Minimum payload length we'll accept before invoking the parser. + * + * For commands declared with `encodeMissingFieldsBehavior: 'skip'`, trailing + * fields may be intentionally omitted (e.g. OTA `hardwareVersion` is only + * present when its `fieldControl` bit is set). We can't derive the strict + * minimum from the struct alone, but every such command in practice has a + * mandatory leading discriminator (status, fieldControl, payloadType) that + * determines which subsequent fields are present. Without that leading byte, + * the parsed result is meaningless; with it, the parser correctly omits + * optional trailing fields. + * + * So for 'skip' commands we require at least the first field's byte width. + * For non-'skip' commands we require the full fixed-portion length. + * + * @param {object} argsType - The ZCLStruct class for this command's args. + * @param {boolean} skipMode - Whether the command uses `encodeMissingFieldsBehavior: 'skip'`. + * @returns {number} Minimum byte length the payload must carry. + */ +function computeMinLength(argsType, skipMode) { + if (!skipMode) { // 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'); - } + return Math.abs(argsType.length); + } + + const firstField = Object.values(argsType.fields)[0]; + if (!firstField) return 0; + // Fixed-width field's length is a positive integer; varsize is negative or 0. + // For varsize first fields (uncommon for 'skip' commands), require at least + // 1 byte so the length prefix is present. + return firstField.length > 0 ? firstField.length : 1; +} + +function parseCommandArgs(command, data) { + const argsType = command.args; + const skipMode = command.encodeMissingFieldsBehavior === 'skip'; + + const minLength = computeMinLength(argsType, skipMode); + if (data.length < minLength) { + throw new ZCLError('MALFORMED_COMMAND'); } try { diff --git a/lib/zclFrames.js b/lib/zclFrames.js index 9b0c394..8491b37 100644 --- a/lib/zclFrames.js +++ b/lib/zclFrames.js @@ -1,6 +1,7 @@ 'use strict'; const { ZCLDataType, ZCLDataTypes, ZCLStruct } = require('./zclTypes'); +const { ZCLError } = require('./util'); const ZCLFrameControlBitmap = ZCLDataTypes.map8( 'clusterSpecific', @@ -84,6 +85,16 @@ const ZCLAttributeDataRecord = (withStatus, attributes) => res.name = attributes[res.id].name || `unknown_attr_${res.id}`; } + // Validate that enough bytes remain for the value field before parsing. + // Without this, fixed-width primitives in @athombv/data-types silently + // return 0 on short input - which for IAS Zone `zoneStatus` (map16) + // produces an all-bits-false bitmap that looks like "all alarms clear", + // and for any status-bearing attribute promotes a truncated value into + // a SUCCESS-looking enum. + if (DataType.length > 0 && buf.length - i < DataType.length) { + throw new ZCLError('MALFORMED_COMMAND'); + } + const entry = DataType.fromBuffer(buf, i, true); if (DataType.length > 0) { i += DataType.length; diff --git a/test/testMalformedFrames.js b/test/testMalformedFrames.js index c0fd957..6d6f999 100644 --- a/test/testMalformedFrames.js +++ b/test/testMalformedFrames.js @@ -1,176 +1,100 @@ 'use strict'; -const { describe, it } = require('node:test'); +const { describe, it, mock } = require('node:test'); const assert = require('node: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 }; +const OTACluster = require('../lib/clusters/ota'); +const { createVirtualDevice, frameToClient } = require('./util/virtualDevice'); + +const DEFAULT_RESPONSE = 0x0b; +const REPORT_ATTRIBUTES = 0x0a; +const MALFORMED_COMMAND = 0x80; +const MAP16 = 0x19; +const ZCL_HEADER_SIZE = 3; + +/** The payload, [cmdId, status], of the Default Response `from` sent for `cmdId`. */ +function defaultResponse(frames, from, cmdId) { + const record = frames.find( + ({ from: sender, frame }) => + sender === from && + frame.cmdId === DEFAULT_RESPONSE && + !frame.frameControl.clusterSpecific && + frame.data[0] === cmdId, + ); + return record && record.frame.data; } -/** 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), - }; +function createIasZoneSensor() { + return createVirtualDevice({ + endpoints: [{ endpointId: 1, inputClusters: [IASZoneCluster.ID] }], + }); } -describe('Malformed frame hardening', function () { - describe('Cluster.handleFrame (server-to-client direction)', function () { - it('should not invoke handler on truncated zoneStatusChangeNotification', function (t, 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 (t, 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('Malformed frames', function () { + it('rejects a truncated command to a Cluster', async function () { + const { zclNode, virtualDevice, frames, settle } = createIasZoneSensor(); + const handler = mock.fn(); + zclNode.endpoints[1].clusters.iasZone.onZoneStatusChangeNotification = handler; + const { id } = IASZoneCluster.COMMANDS.zoneStatusChangeNotification; + + // zoneStatus only, the other 4 of 6 bytes are missing + const frame = frameToClient({ cmdId: id, clusterSpecific: true, data: Buffer.from([1, 0]) }); + await virtualDevice.sendFrame(1, IASZoneCluster.ID, frame); + await settle(); + + assert.strictEqual(handler.mock.callCount(), 0); + assert.deepStrictEqual( + defaultResponse(frames, 'zclNode', id), + Buffer.from([id, MALFORMED_COMMAND]), + ); }); - 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); + it('rejects an attribute report with a truncated value', async function () { + const { zclNode, virtualDevice, frames, settle } = createIasZoneSensor(); + const onZoneStatus = mock.fn(); + zclNode.endpoints[1].clusters.iasZone.on('attr.zoneStatus', onZoneStatus); + + // zoneStatus is map16, only 1 of its 2 value bytes is sent + const { id } = IASZoneCluster.ATTRIBUTES.zoneStatus; + const frame = frameToClient({ cmdId: REPORT_ATTRIBUTES, data: Buffer.from([id, 0, MAP16, 1]) }); + await virtualDevice.sendFrame(1, IASZoneCluster.ID, frame); + await settle(); + + assert.strictEqual(onZoneStatus.mock.callCount(), 0); + assert.deepStrictEqual( + defaultResponse(frames, 'zclNode', REPORT_ATTRIBUTES), + Buffer.from([REPORT_ATTRIBUTES, MALFORMED_COMMAND]), + ); + }); - 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('rejects an empty command to a BoundCluster that may skip trailing fields', async function () { + const { zclNode, virtualDevice, intercept } = createVirtualDevice({ + endpoints: [{ endpointId: 1, outputClusters: [OTACluster.ID] }], }); + const handler = mock.fn(async () => ({ status: 'NO_IMAGE_AVAILABLE' })); + zclNode.endpoints[1].bind( + 'ota', + new (class extends BoundCluster { + queryNextImageRequest(...args) { + return handler(...args); + } + })(), + ); + // Keep the header, drop the payload + intercept(({ from, raw }) => + from === 'virtualDevice' ? [raw.subarray(0, ZCL_HEADER_SIZE)] : undefined, + ); + + await assert.rejects( + virtualDevice.endpoints[1].clusters.ota.queryNextImageRequest({ + fieldControl: 0, + manufacturerCode: 0x1234, + imageType: 1, + fileVersion: 1, + }), + /^Error: MALFORMED_COMMAND$/, + ); + assert.strictEqual(handler.mock.callCount(), 0); }); });