Skip to content

fix(services): decode GetEventInformation without object identifier - #190

Open
chowchin wants to merge 1 commit into
fh1ch:masterfrom
chowchin:fix/get-event-information-optional-object-id
Open

fix(services): decode GetEventInformation without object identifier#190
chowchin wants to merge 1 commit into
fh1ch:masterfrom
chowchin:fix/get-event-information-optional-object-id

Conversation

@chowchin

Copy link
Copy Markdown

Problem

lastReceivedObjectIdentifier in a GetEventInformation request is optional (ASHRAE 135, 13.12.1.1). A client asking for the first batch of events omits it, so the request carries no service payload — but getEventInformation.decode reads the identifier unconditionally and runs past the end of the buffer.

The RangeError is raised inside the dgram message handler (_receiveData_handleNpdu_handlePdu_processConfirmedServiceRequest), which application code cannot wrap, so one such request terminates the process of any device using bacstack to serve BACnet requests.

Reproduction — a request with no payload is a 10-byte datagram (BVLC 4 + NPDU 2 + APDU header 4), so client.js calls decode(buffer, 10, 0):

const service = require('./lib/services/get-event-information');
service.decode(Buffer.alloc(10), 10, 0);
master        : throws ERR_OUT_OF_RANGE - The value of "offset" is out of range. It must be >= 0 and <= 6. Received 11
with this fix : {"lastReceivedObjectId":null,"len":0}

Observed in production against a BMS that polls for alarms: every scan killed the gateway process, the service manager restarted it, the BMS retried and killed it again.

RangeError [ERR_OUT_OF_RANGE]: The value of "offset" is out of range. It must be >= 0 and <= 6. Received 11
    at Uint8Array.readUInt32BE (node:internal/buffer:309:5)
    at Object.decodeObjectId (lib/asn1.js:622:18)
    at Object.decode (lib/services/get-event-information.js:15:31)
    at Client._processConfirmedServiceRequest (lib/client.js:242:47)
    at Client._handlePdu (lib/client.js:413:16)
    at Client._handleNpdu (lib/client.js:437:10)

Change

Decode the identifier only when it is actually present, using the apduLen that client.js already passes, plus a check that the next tag really is context tag 0. When it is absent, lastReceivedObjectId is null and len is 0.

The third parameter is optional, so existing callers that pass only (buffer, offset) — including the current unit test — behave exactly as before.

Tests

Two cases added to test/unit/service-get-event-information.spec.js: a request without the optional identifier, and one whose buffer ends exactly at the offset. npm test passes (33 suites, 119 tests).

The lastReceivedObjectIdentifier of a GetEventInformation request is optional
(ASHRAE 135, 13.12.1.1). A client requesting the first batch of events omits it,
which results in a service request without any payload, but the decoder read the
identifier unconditionally and therefore ran past the end of the buffer.

The resulting RangeError is raised inside the transport message handler, where
application code cannot catch it, so a single such request terminates the
process of any device using this library to serve BACnet requests.

Decode the identifier only when it is actually present.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant