From 812ab7d56e0b737fee67d2871c0ff01e1d0ca18e Mon Sep 17 00:00:00 2001 From: James Golovich Date: Sat, 25 Jul 2026 22:46:54 -0700 Subject: [PATCH 1/2] Validate quantity and address range in read request handlers to prevent DoS The server response handler did not validate the quantity field in read requests (FC01-FC04) against the Modbus specification limits. For FC03/FC04 (Read Holding/Input Registers), a quantity greater than 127 produces a byte count exceeding 255, which causes Buffer.writeUInt8() to throw an uncaught RangeError that crashes the Node.js process. A single crafted packet from an unauthenticated client is sufficient to terminate the server. Add quantity range validation per the Modbus spec: - FC01/FC02 (Read Coils/Discrete Inputs): 1-2000 (0x0001-0x07D0) - FC03/FC04 (Read Holding/Input Registers): 1-125 (0x0001-0x007D) Also add address bounds checks for FC03/FC04 to return a Modbus exception (error code 0x02) when start + count exceeds the register buffer, instead of reading out of bounds. Out-of-range requests now receive a proper Modbus exception response (error code 0x03, Illegal Data Value) matching the pattern used by the existing write handlers. --- src/modbus-server-response-handler.ts | 54 +++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/modbus-server-response-handler.ts b/src/modbus-server-response-handler.ts index 6db7062..bb5856b 100644 --- a/src/modbus-server-response-handler.ts +++ b/src/modbus-server-response-handler.ts @@ -109,6 +109,15 @@ export default class ModbusServerResponseHandler 0x7D0) { + debug('illegal data value, quantity out of range: %d', request.body.count) + const exceptionBody = new ExceptionResponseBody(request.body.fc, 0x03) + const exceptionResponse = this._fromRequest(request, exceptionBody) + cb(exceptionResponse.createPayload()) + return exceptionResponse + } + this._server.emit('preReadCoils', request, cb) const responseBody = ReadCoilsResponseBody.fromRequest(request.body, this._server.coils) @@ -133,6 +142,15 @@ export default class ModbusServerResponseHandler 0x7D0) { + debug('illegal data value, quantity out of range: %d', request.body.count) + const exceptionBody = new ExceptionResponseBody(request.body.fc, 0x03) + const exceptionResponse = this._fromRequest(request, exceptionBody) + cb(exceptionResponse.createPayload()) + return exceptionResponse + } + this._server.emit('preReadDiscreteInputs', request, cb) const responseBody = ReadDiscreteInputsResponseBody.fromRequest(request.body, this._server.discrete) @@ -192,6 +210,24 @@ export default class ModbusServerResponseHandler 0x7D) { + debug('illegal data value, quantity out of range: %d', request.body.count) + const exceptionBody = new ExceptionResponseBody(request.body.fc, 0x03) + const exceptionResponse = this._fromRequest(request, exceptionBody) + cb(exceptionResponse.createPayload()) + return exceptionResponse + } + + /* Validate start address + count does not exceed holding register buffer */ + if ((request.body.start + request.body.count) * 2 > this._server.holding.length) { + debug('illegal data address') + const exceptionBody = new ExceptionResponseBody(request.body.fc, 0x02) + const exceptionResponse = this._fromRequest(request, exceptionBody) + cb(exceptionResponse.createPayload()) + return exceptionResponse + } + this._server.emit('preReadHoldingRegisters', request, cb) const responseBody = ReadHoldingRegistersResponseBody.fromRequest(request.body, this._server.holding) @@ -216,6 +252,24 @@ export default class ModbusServerResponseHandler 0x7D) { + debug('illegal data value, quantity out of range: %d', request.body.count) + const exceptionBody = new ExceptionResponseBody(request.body.fc, 0x03) + const exceptionResponse = this._fromRequest(request, exceptionBody) + cb(exceptionResponse.createPayload()) + return exceptionResponse + } + + /* Validate start address + count does not exceed input register buffer */ + if ((request.body.start + request.body.count) * 2 > this._server.input.length) { + debug('illegal data address') + const exceptionBody = new ExceptionResponseBody(request.body.fc, 0x02) + const exceptionResponse = this._fromRequest(request, exceptionBody) + cb(exceptionResponse.createPayload()) + return exceptionResponse + } + this._server.emit('preReadInputRegisters', request, cb) const responseBody = ReadInputRegistersResponseBody.fromRequest(request.body, this._server.input) From 1640a807adf18d9c608665bcbe8cff64a82c6ef6 Mon Sep 17 00:00:00 2001 From: James Golovich Date: Sat, 25 Jul 2026 22:52:31 -0700 Subject: [PATCH 2/2] Add tests for read request quantity and address validation Tests cover: - FC01/FC02: reject quantity of 0 (below minimum) - FC03/FC04: reject quantity > 125 (Modbus spec limit) - FC03: reject quantity of 0 - FC03/FC04: reject start + count exceeding register buffer (error 0x02) - FC03: DoS regression test using the exact payload from the vulnerability disclosure (quantity 617, byte count 1234 overflows UInt8) - FC03: valid request at max buffer capacity still succeeds Also adds input and discrete buffers to the test server setup to enable FC02 and FC04 testing. --- test/tcp-server.test.js | 250 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 249 insertions(+), 1 deletion(-) diff --git a/test/tcp-server.test.js b/test/tcp-server.test.js index 96cefe5..bd20d6b 100644 --- a/test/tcp-server.test.js +++ b/test/tcp-server.test.js @@ -14,7 +14,9 @@ describe('TCP Server Tests.', function () { server = new Modbus.server.TCP(socket, { holding: Buffer.alloc(12, 0x00), - coils: Buffer.from([0x55, 0x55, 0x55]) + input: Buffer.alloc(12, 0x00), + coils: Buffer.from([0x55, 0x55, 0x55]), + discrete: Buffer.from([0xAA, 0xAA, 0xAA]) }) }) @@ -320,4 +322,250 @@ describe('TCP Server Tests.', function () { socket.emit('data', request) }) }) + + describe('Read Request Quantity Validation Tests.', function () { + it('should return exception 0x03 for FC01 read coils with quantity of 0', function (done) { + const request = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x06, // byte count + 0x02, // unit id + 0x01, // function code + 0x00, 0x00, // starting address + 0x00, 0x00 // quantity 0 (below minimum of 1) + ]) + const expectedResponse = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x03, // byte count + 0x02, // unit id + 0x81, // exception for FC01 + 0x03 // ILLEGAL DATA VALUE + ]) + + socket.write = (response) => { + assert.deepEqual(expectedResponse, response) + done() + } + + socket.emit('connection', socket) + socket.emit('data', request) + }) + + it('should return exception 0x03 for FC02 read discrete inputs with quantity of 0', function (done) { + const request = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x06, // byte count + 0x02, // unit id + 0x02, // function code + 0x00, 0x00, // starting address + 0x00, 0x00 // quantity 0 (below minimum of 1) + ]) + const expectedResponse = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x03, // byte count + 0x02, // unit id + 0x82, // exception for FC02 + 0x03 // ILLEGAL DATA VALUE + ]) + + socket.write = (response) => { + assert.deepEqual(expectedResponse, response) + done() + } + + socket.emit('connection', socket) + socket.emit('data', request) + }) + + it('should return exception 0x03 for FC03 read holding registers with quantity > 125', function (done) { + const request = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x06, // byte count + 0x02, // unit id + 0x03, // function code + 0x00, 0x00, // starting address + 0x00, 0x7E // quantity 126 (exceeds 125 limit) + ]) + const expectedResponse = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x03, // byte count + 0x02, // unit id + 0x83, // exception for FC03 + 0x03 // ILLEGAL DATA VALUE + ]) + + socket.write = (response) => { + assert.deepEqual(expectedResponse, response) + done() + } + + socket.emit('connection', socket) + socket.emit('data', request) + }) + + it('should return exception 0x03 for FC04 read input registers with quantity > 125', function (done) { + const request = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x06, // byte count + 0x02, // unit id + 0x04, // function code + 0x00, 0x00, // starting address + 0x00, 0x7E // quantity 126 (exceeds 125 limit) + ]) + const expectedResponse = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x03, // byte count + 0x02, // unit id + 0x84, // exception for FC04 + 0x03 // ILLEGAL DATA VALUE + ]) + + socket.write = (response) => { + assert.deepEqual(expectedResponse, response) + done() + } + + socket.emit('connection', socket) + socket.emit('data', request) + }) + + it('should return exception 0x03 for FC03 with quantity of 0', function (done) { + const request = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x06, // byte count + 0x02, // unit id + 0x03, // function code + 0x00, 0x00, // starting address + 0x00, 0x00 // quantity 0 (below minimum of 1) + ]) + const expectedResponse = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x03, // byte count + 0x02, // unit id + 0x83, // exception for FC03 + 0x03 // ILLEGAL DATA VALUE + ]) + + socket.write = (response) => { + assert.deepEqual(expectedResponse, response) + done() + } + + socket.emit('connection', socket) + socket.emit('data', request) + }) + + it('should return exception 0x02 for FC03 when start + count exceeds holding buffer', function (done) { + const request = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x06, // byte count + 0x02, // unit id + 0x03, // function code + 0x00, 0x04, // starting address 4 + 0x00, 0x04 // quantity 4 (address 4 + 4 = 8 registers = 16 bytes > 12 byte buffer) + ]) + const expectedResponse = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x03, // byte count + 0x02, // unit id + 0x83, // exception for FC03 + 0x02 // ILLEGAL DATA ADDRESS + ]) + + socket.write = (response) => { + assert.deepEqual(expectedResponse, response) + done() + } + + socket.emit('connection', socket) + socket.emit('data', request) + }) + + it('should return exception 0x02 for FC04 when start + count exceeds input buffer', function (done) { + const request = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x06, // byte count + 0x02, // unit id + 0x04, // function code + 0x00, 0x04, // starting address 4 + 0x00, 0x04 // quantity 4 (address 4 + 4 = 8 registers = 16 bytes > 12 byte buffer) + ]) + const expectedResponse = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x03, // byte count + 0x02, // unit id + 0x84, // exception for FC04 + 0x02 // ILLEGAL DATA ADDRESS + ]) + + socket.write = (response) => { + assert.deepEqual(expectedResponse, response) + done() + } + + socket.emit('connection', socket) + socket.emit('data', request) + }) + + it('should not crash when FC03 quantity would overflow byte count (DoS regression)', function (done) { + const request = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x06, // byte count + 0x02, // unit id + 0x03, // function code + 0x05, 0x39, // starting address 1337 + 0x02, 0x69 // quantity 617 (byte count 1234 overflows UInt8) + ]) + const expectedResponse = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x03, // byte count + 0x02, // unit id + 0x83, // exception for FC03 + 0x03 // ILLEGAL DATA VALUE + ]) + + socket.write = (response) => { + assert.deepEqual(expectedResponse, response) + done() + } + + socket.emit('connection', socket) + socket.emit('data', request) + }) + + it('should accept FC03 read holding registers at max valid quantity for buffer', function (done) { + const request = Buffer.from([ + 0x00, 0x01, // transaction id + 0x00, 0x00, // protocol + 0x00, 0x06, // byte count + 0x02, // unit id + 0x03, // function code + 0x00, 0x00, // starting address 0 + 0x00, 0x06 // quantity 6 (exactly fills 12-byte holding buffer) + ]) + + socket.write = (response) => { + assert.equal(response.readUInt8(7), 0x03) + done() + } + + socket.emit('connection', socket) + socket.emit('data', request) + }) + }) })