Open raw-phil opened 7 months ago
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?
@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.
@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 🤔
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.
@kamilmysliwiec I agree with you. In the next days I'll work on the PR. Thanks
I have noticed that the length used before the delimiter character(#) represents the number of characters sent instead of the bytes of the message.
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
Is there an existing issue for this?
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 methodhandleData
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.https://github.com/nestjs/nest/blob/d6a6f30165eb021797f6162e5bf7f4567b0189fa/packages/microservices/helpers/json-socket.ts#L17-L40
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
:Run the app:
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:
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
Other package
No response
NestJS version
10.3.1
Packages versions
Node.js version
20.1.0
In which operating systems have you tested?
Other
No response