You said:
ChatGPT said:
I’ll treat the uploaded nats.js-main.zip as the target and look for correctness/security hazards rather than style issues. I’ll scan the repo structure, dependency/test surface, and high-risk code paths, then report only findings that look plausibly serious.
The repo is a multi-package TypeScript/Deno workspace at version 3.4.0. I’m prioritizing protocol parsing, auth/TLS, JetStream/ObjectStore/KV semantics, and publish/request memory behavior; test helper code is lower priority unless it ships.
Yes. I found two definite serious correctness issues, plus two security/robustness issues worth fixing before release.
Scope: static review of the uploaded nats.js-main.zip. I did not run the full test suite because deno is not installed in this container. I also checked the official NATS advisory index; recent listed advisories are mostly nats-server, while the JS-client item visible there is the older CVE-2020-26149 information-disclosure issue. NATS Advisories
1. Header support check constructs an error but does not throw it
File: core/src/protocol.ts:970-976
TypeScriptif (options.headers) {
if (this.info && !this.info.headers) {
InvalidArgumentError.format(
"headers",
"are not available on this server",
);
}
// ...
}
This should almost certainly be:
TypeScriptthrow InvalidArgumentError.format(...);
Current behavior: if connected to a server that does not advertise header support, the client still sends HPUB at core/src/protocol.ts:989-996. That can produce protocol errors/disconnects instead of a local validation failure.
Severity: high correctness/compatibility bug.
Fix: throw the error and add a regression test against info.headers === false.
2. ObjectStore digest mismatch can resolve ObjectResult.error successfully
Files: obj/src/objectstore.ts:732-760, obj/src/types.ts:216-220
The type contract says:
TypeScripterror: Promise<Error | null>;
and documents it as the promise to check after reading the stream. But on checksum mismatch:
TypeScriptcontroller!.error(
new Error(
`received a corrupt object, digests do not match\n expected: ${info.digest}\n computed: SHA-256 ${hex}`,
),
);
then the loop breaks, the async function resolves, and .then(() => d.resolve()) runs.
So the ReadableStream errors, but ObjectResult.error can resolve successfully rather than reporting the corruption. getBlob() probably still rejects because it consumes the errored stream, but callers using the documented .error path can miss integrity failure.
Severity: high data-integrity/API-contract bug.
Fix: store the digest error, call controller.error(err), then reject/resolve the promise with that error consistently. For example:
TypeScriptconst err = new Error(...);
controller!.error(err);
throw err;
or explicitly d.resolve(err) if the intended contract is “resolve to error” rather than reject.
3. Parser errors are swallowed by the read loop
File: core/src/protocol.ts:578-586
TypeScript(async () => {
try {
for await (const b of this.transport) {
this.parser.parse(b);
}
} catch (err) {
console.log("reader closed", err);
}
})().then();
If parser.parse() throws on malformed protocol, the reader task just logs and exits. It does not close the connection, set lastError, reject pending pings/requests, emit a status event, or reconnect immediately.
A malicious/buggy server or corrupt proxy can leave the client half-alive until heartbeat/stale detection catches up. That is especially bad for request/reply users, because requests can hang until their own timeouts rather than failing promptly.
Severity: medium-high robustness issue; more serious if clients connect to untrusted endpoints.
Fix: treat parser failure as fatal transport failure: close/disconnect/reconnect through the same path used for transport errors, and do not use unconditional console.log in library code.
4. Msg.respond(..., { reply }) bypasses public publish validation
Files: core/src/msg.ts:79-85, core/src/protocol.ts:948-1004, core/src/nats.ts:101-109
The public NatsConnectionImpl.publish() validates both subject and options.reply:
TypeScriptthis._check(subject, false, true);
if (options?.reply) {
this._check(options.reply, false, true);
}
But MsgImpl.respond() calls the lower-level protocol publisher directly:
TypeScriptthis.publisher.publish(this.reply, data, opts);
and ProtocolHandler.publish() interpolates options.reply into the protocol string without validation:
TypeScriptproto = `PUB ${subject} ${options.reply} ${len}\r\n`;
this.reply itself is normally server-provided and tokenized, so the obvious subject is not the issue. The issue is user/application-supplied opts.reply. If application code reflects attacker-controlled input into msg.respond(data, { reply }), whitespace/CRLF can become protocol injection.
Severity: medium security footgun. Not a standalone remote exploit unless the application passes untrusted reply, but the low-level path is weaker than the public API.
Fix: centralize validation inside ProtocolHandler.publish() so every call path is protected, or validate opts.reply in Msg.respond() with the same rules as NatsConnectionImpl.publish().
Also worth checking
transport-deno/src/deno_transport.ts:73-80 ignores hp.tlsName in the handshakeFirst === true path and uses hp.hostname for TLS options/SNI/verification. The non-handshake-first path uses tlsName at transport-deno/src/deno_transport.ts:93-95. If DNS resolution rewrites hostname to an IP, Deno TLS-first connections can fail certificate hostname verification or send the wrong SNI. That is likely an availability/platform bug, not a security bypass.
Triage order
Patch #1 and #2 first; they are definite bugs with small fixes. Then fix #3 because parser exceptions should not leave the connection in an ambiguous state. Fix #4 by moving validation down into ProtocolHandler.publish() so future internal call paths cannot bypass it.