TCP microservice does NOT have a max message length

[raw-phil]

Is there an existing issue for this?

• I have searched the existing issues

Current behavior

From what I've got about nest TCP microservice, the JsonSocket class, in packages/microservices/helpers/json-socket.ts, handle json messages received from a TCP microservice.
The method handleData take data received from the TCP stream, and use the byte length sent before the delimiter character to determine where the json starts and ends.

nest/packages/microservices/helpers/json-socket.ts

Lines 17 to 40 in d6a6f30

	protected handleData(dataRaw: Buffer | string) {
	const data = Buffer.isBuffer(dataRaw)
	? this.stringDecoder.write(dataRaw)
	: dataRaw;
	this.buffer += data;
	if (this.contentLength == null) {
	const i = this.buffer.indexOf(this.delimiter);
	/**
	* Check if the buffer has the delimiter (#),
	* if not, the end of the buffer string might be in the middle of a content length string
	*/
	if (i !== -1) {
	const rawContentLength = this.buffer.substring(0, i);
	this.contentLength = parseInt(rawContentLength, 10);
	if (isNaN(this.contentLength)) {
	this.contentLength = null;
	this.buffer = '';
	throw new CorruptedPacketLengthException(rawContentLength);
	}
	this.buffer = this.buffer.substring(i + 1);
	}
	}

Bug:

The problem is that there is no check for a possible message max size.
If a malicious user send a continuous stream of random data, without a single '#' inside, the buffer will never be emptied and will fill up until the max length for a string in nodeJS is reached.
The nest app will start to consume huge amount of cpu resources and memory. Even worse if the attacker is sending data through multiple connections.

Minimum reproduction code

https://github.com/raw-phil/nest-tcp-microservice

Steps to reproduce

The app main.ts:

async function bootstrap() {
  const port = 1234;
  const app = await NestFactory.createMicroservice(AppModule, {
    transport: Transport.TCP,
    options: {
      host: '127.0.0.1',
      port,
    },
  });
  await app.listen();
  console.log('Microservice listening on port:', port);
}
bootstrap();

Run the app:

$ npm install

# production mode
$ npm run start:prod

Then open a TCP connection to localhost:1234 and send an endless stream of any data except the # character, that is the delimiter.

For example in bash:

 $ yes 1 | nc -v localhost 1234

Expected behavior

In JsonSocket class, packages/microservices/helpers/json-socket.ts -> handleData method.
I expected that the data received before the delimiter character, are checked to find if they are numbers and so a possible length field. And that there should be a limit to the length field and the message received.

Package

• I don't know. Or some 3rd-party package
• @nestjs/common
• @nestjs/core
• @nestjs/microservices
• @nestjs/platform-express
• @nestjs/platform-fastify
• @nestjs/platform-socket.io
• @nestjs/platform-ws
• @nestjs/testing
• @nestjs/websockets
• Other (see below)

Other package

No response

NestJS version

10.3.1

Packages versions

{
  "name": "nest-tcp-microservice",
  "version": "0.0.1",
  "description": "",
  "author": "",
  "private": true,
  "license": "UNLICENSED",
  "scripts": {
    "build": "nest build",
    "format": "prettier --write \"src/**/*.ts\" \"test/**/*.ts\"",
    "start": "nest start",
    "start:dev": "nest start --watch",
    "start:debug": "nest start --debug --watch",
    "start:prod": "node dist/main",
    "lint": "eslint \"{src,apps,libs,test}/**/*.ts\" --fix",
    "test": "jest",
    "test:watch": "jest --watch",
    "test:cov": "jest --coverage",
    "test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand",
    "test:e2e": "jest --config ./test/jest-e2e.json"
  },
  "dependencies": {
    "@nestjs/common": "^10.0.0",
    "@nestjs/core": "^10.0.0",
    "@nestjs/platform-express": "^10.0.0",
    "reflect-metadata": "^0.1.13",
    "rxjs": "^7.8.1"
  },
  "devDependencies": {
    "@nestjs/cli": "^10.0.0",
    "@nestjs/schematics": "^10.0.0",
    "@nestjs/testing": "^10.0.0",
    "@types/express": "^4.17.17",
    "@types/jest": "^29.5.2",
    "@types/node": "^20.3.1",
    "@types/supertest": "^2.0.12",
    "@typescript-eslint/eslint-plugin": "^5.59.11",
    "@typescript-eslint/parser": "^5.59.11",
    "eslint": "^8.42.0",
    "eslint-config-prettier": "^8.8.0",
    "eslint-plugin-prettier": "^4.2.1",
    "jest": "^29.5.0",
    "prettier": "^2.8.8",
    "source-map-support": "^0.5.21",
    "supertest": "^6.3.3",
    "ts-jest": "^29.1.0",
    "ts-loader": "^9.4.3",
    "ts-node": "^10.9.1",
    "tsconfig-paths": "^4.2.0",
    "typescript": "^5.1.3"
  },
  "jest": {
    "moduleFileExtensions": [
      "js",
      "json",
      "ts"
    ],
    "rootDir": "src",
    "testRegex": ".*\\.spec\\.ts$",
    "transform": {
      "^.+\\.(t|j)s$": "ts-jest"
    },
    "collectCoverageFrom": [
      "**/*.(t|j)s"
    ],
    "coverageDirectory": "../coverage",
    "testEnvironment": "node"
  }
}

Node.js version

20.1.0

In which operating systems have you tested?

• macOS
• Windows
• Linux

Other

No response

[kamilmysliwiec]

TCP microservices should never be exposed as public services so there's no risk of anyone sending malicious data.

Still, we should fix this issue either way. Would you like to create a PR for this?

[raw-phil]

@kamilmysliwiec sure I'd like to create a PR, I just don't know how big the maximum size needs to be set for incoming json message. Maybe to the max string size of V8 engine, that if I remember correctly is 1GB.

[underfisk]

@raw-phil I would say that it could be adjustable, the developer might want to specify whatever value he has in mind as sometimes it's hard to accurately read the value. Applying the default based on the max string for v8 engine could work but allowing the developer to override should be possible 🤔

[kamilmysliwiec]

IMO even the 512 MB setting is much more than needed. We could keep it this high though just to avoid introducing another configuration option.

[raw-phil]

@kamilmysliwiec I agree with you. In the next days I'll work on the PR. Thanks

[raw-phil]

I have noticed that the length used before the delimiter character(#) represents the number of characters sent instead of the bytes of the message.

nest/packages/microservices/helpers/json-socket.ts

Lines 61 to 66 in d6a6f30

	private formatMessageData(message: any) {
	const messageData = JSON.stringify(message);
	const length = messageData.length;
	const data = length + this.delimiter + messageData;
	return data;
	}

I've done a couple of researches, and JS use the UTF-16 encoding which is variable-length, code points are encoded with one or two 16-bit code units.
IMO, if we want to set the maximum string/buffer size to 512MB, we have to consider the worst case where all characters need 32 bits to be encoded, and then I think that the maximum length (in characters) of the message should be set to 512MB / 32 bit = 134217728, or something like that