authenio / samlify-node-xmllint Goto Github PK

At the beginning, I will mention that I'm aware that the main source of the issue is node-xmllint however it looks that node-xmllint is not maintained anymore, so any fix there would require creating a fork or merging its code directly here.

I noticed in my application that handle SAML that I got node warnings:
(node:1) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 uncaughtException listeners added to [process]. Use emitter.setMaxListeners() to increase limit

After deeper investigations, I found that uncaughtException listeners are registered by node-xmllint called by samlify-node-xmllint.
node-xmllint has been made as a conversion of libxml2 using Emscripten. Emscripten contains a feature that it auto-register these handlers for each compilation. In the newer version, it is possible to disable it:
emscripten-core/emscripten#7855

Why is this problematic?

• Library produce undocumented side effect
• As a registered handler is terminating the application, it might block other custom handlers from running (for example user might want to connect the logger to this event before closing the app, which might not be called due to handlers registered by node-xmllint)
• Handler is registered with each reference to this library which might lead to memory leaks.

this pacakge depends on node-xmllint, when validate XML with node-xmllint.validateXML. it will register a drain event in process.stdout to exit the current process. but if no drain event emitted in current express request, this event did not unregister. once log a big data somewhere, the registered event will be emitted, this will make node process exit with code 0.

stacktrace

at exit (node_modules/.pnpm/[email protected]/node_modules/node-xmllint/xmllint.js:242550:11)
at Object.callMain (node_modules/.pnpm/[email protected]/node_modules/node-xmllint/xmllint.js:242496:4)
at doRun (node_modules/.pnpm/[email protected]/node_modules/node-xmllint/xmllint.js:242532:59)
at run (node_modules/.pnpm/[email protected]/node_modules/node-xmllint/xmllint.js:242544:4)
at Object.xmllint.validateXML (node_modules/.pnpm/[email protected]/node_modules/node-xmllint/xmllint.js:242592:2)
at node_modules/.pnpm/@[email protected][email protected]/node_modules/@authenio/samlify-node-xmllint/index.ts:15:34
at new Promise (<anonymous>)
at exports.validate (node_modules/.pnpm/@[email protected][email protected]/node_modules/@authenio/samlify-node-xmllint/index.ts:14:10)
at Object.<anonymous> (node_modules/.pnpm/[email protected]/node_modules/samlify/src/libsaml.ts:708:22)
at step (node_modules/.pnpm/[email protected]/node_modules/samlify/build/src/libsaml.js:61:23)
at Object.next (node_modules/.pnpm/[email protected]/node_modules/samlify/build/src/libsaml.js:42:53)
at node_modules/.pnpm/[email protected]/node_modules/samlify/build/src/libsaml.js:36:71
at new Promise (<anonymous>)
at __awaiter (node_modules/.pnpm/[email protected]/node_modules/samlify/build/src/libsaml.js:32:12)
at Object.isValidXml (node_modules/.pnpm/[email protected]/node_modules/samlify/build/src/libsaml.js:588:20)
at node_modules/.pnpm/[email protected]/node_modules/samlify/src/flow.ts:201:17
at step (node_modules/.pnpm/[email protected]/node_modules/samlify/build/src/flow.js:33:23)
at Object.next (node_modules/.pnpm/[email protected]/node_modules/samlify/build/src/flow.js:14:53)
at node_modules/.pnpm/[email protected]/node_modules/samlify/build/src/flow.js:8:71
at new Promise (<anonymous>)
at __awaiter (node_modules/.pnpm/[email protected]/node_modules/samlify/build/src/flow.js:4:12)
at postFlow (node_modules/.pnpm/[email protected]/node_modules/samlify/build/src/flow.js:181:12)
at flow (node_modules/.pnpm/[email protected]/node_modules/samlify/src/flow.ts:456:12)

// package node-xmllint/xmllint.js (after formatted, because the source code in github is unavailable)
function exit(status) {
  console.trace("xmllint track exit called", {status})
  if (Module["noExitRuntime"]) {
           return
  }
  ABORT = true;
  EXITSTATUS = status;
  STACKTOP = initialStackTop;
  exitRuntime();
  if (ENVIRONMENT_IS_NODE) {
	  process["stdout"]["once"]("drain", (function () { // ========> register drain event here
		  console.trace("xmllint drain");
		  process["exit"](status)  // ========> exit current process
	  }));
	  console.log(" ");
  } else if (ENVIRONMENT_IS_SHELL && typeof quit === "function") {
	  quit(status)
  }
  throw new ExitStatus(status)
  }

flow:
samlify-node-xmllint.validate -> node-xmllint.validateXML (xmllint.js) -> node-xmllint.run (xmllint.js) -> node-xmllint.doRun (xmllint.js) -> node-xmllint.callMain (xmllint.js) -> node-xmllint.exit (xmllint.js)

Env:
Node: 18
OS: debian 12
Framework: Express
samlify: 2.8.6
authenio/samlify-node-xmllint: 2.0.0

Each call of validate function will consume about 16MB! of array buffer memory. Node 18.

const SAMLValidator = require('@authenio/samlify-node-xmllint');

const XML = `<samlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
    ID="aaf23196-1773-2113-474a-fe114412ab72"
    Version="2.0"
    IssueInstant="2004-12-05T09:21:59Z">
</samlp:AuthnRequest>`;

console.log(process.memoryUsage());
SAMLValidator.validate(XML);
console.log(process.memoryUsage());
SAMLValidator.validate(XML);
console.log(process.memoryUsage());

Memory before call:

{
  rss: 41086976,
  heapTotal: 14307328,
  heapUsed: 9071328,
  external: 406411,
  arrayBuffers: 16610
}

Memory after 2 calls:

{
  rss: 58294272,
  heapTotal: 18771968,
  heapUsed: 13314016,
  external: 34080083,
  arrayBuffers: 33654784
}

When bundling with webpack, the /schemas directory tends to get left behind. Depending on the bundle, there's no easy way to include those files in the build every time because they're being loaded via fs. This is especially tricky on a serverless environment (AWS, etc).

Alternatives to the current method could include making those files actual ts or js files and using await import to dynamically import the schema that's needed.