fix: secure header assignment to avoid process crash

* fixes [Issue #323](#323)
moleculerjs · Aug 28, 2023 · 0e07cd1 · 0e07cd1
1 parent 7963fb4
commit 0e07cd1
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 10 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/src/index.js b/src/index.js
@@ -746,7 +746,12 @@ module.exports = {
 					if (key == "Content-Type" && !responseType)
 						responseType = ctx.meta.$responseHeaders[key];
 					else
-						res.setHeader(key, ctx.meta.$responseHeaders[key]);
+						try {
+							res.setHeader(key, ctx.meta.$responseHeaders[key]);
+						} catch (error) {
+							this.logger.warn("Invalid header value", req.url, error);
+							res.setHeader(key, encodeURI(ctx.meta.$responseHeaders[key]));
+						}
 				});
 			}
 			if (data == null)
@@ -898,7 +903,12 @@ module.exports = {
 						if (key === "Content-Type" && !responseType)
 							responseType = ctx.meta.$responseHeaders[key];
 						else
-							res.setHeader(key, ctx.meta.$responseHeaders[key]);
+							try {
+								res.setHeader(key, ctx.meta.$responseHeaders[key]);
+							} catch (error) {
+								this.logger.warn("Invalid header value", req.url, error);
+								res.setHeader(key, encodeURI(ctx.meta.$responseHeaders[key]));
+							}
 					});
 				}
 			}

diff --git a/test/integration/index.spec.js b/test/integration/index.spec.js
@@ -387,7 +387,7 @@ describe("Test responses", () => {
 			.then(res => {
 				expect(res.statusCode).toBe(500);
 				expect(res.headers["content-type"]).toBe("application/json; charset=utf-8");
-				expect(res.header["x-request-id"]).toBeDefined();
+				expect(res.headers["x-request-id"]).toBeDefined();
 				expect(res.body).toEqual({
 					"code": 500,
 					"message": "I'm dangerous",
@@ -402,11 +402,22 @@ describe("Test responses", () => {
 			.then(res => {
 				expect(res.statusCode).toBe(500);
 				expect(res.headers["content-type"]).toBe("text/plain");
-				expect(res.header["x-request-id"]).toBeDefined();
-				expect(res.header["x-custom-header"]).toBe("Custom content");
+				expect(res.headers["x-invalid-header"]).toBe(encodeURI("\r\nBOOM"));
+				expect(res.headers["x-request-id"]).toBeDefined();
+				expect(res.headers["x-custom-header"]).toBe("Custom content");
 				expect(res.text).toBe("{\"name\":\"MoleculerServerError\",\"message\":\"It is a wrong action! I always throw error!\",\"code\":500}");
 			});
 	});
+
+	it("GET /test/invalidResponseHeaders", () => {
+		return request(server)
+			.get("/test/invalidResponseHeaders")
+			.then(res => {
+				expect(res.headers["x-valid-header"]).toBe("valid header");
+				expect(res.headers["x-invalid-header"]).toBe(encodeURI("\r\nBOOM"));
+				expect(res.text).toBe("{\"x-invalid-header\":\"\\r\\nBOOM\"}");
+			});
+	});
 });
 
 describe("Test with `path` prefix", () => {
@@ -1626,7 +1637,7 @@ describe("Test REST shorthand aliases and except filter", () => {
 	});
 });
 
-describe("Test REST shorthand aliases and only, execpt filter", () => {
+describe("Test REST shorthand aliases and only, except filter", () => {
 	let broker;
 	let service;
 	let server;

diff --git a/test/services/test.service.js b/test/services/test.service.js
@@ -281,12 +281,24 @@ module.exports = {
 		errorWithHeader(ctx) {
 			ctx.meta.$responseType = "text/plain";
 			ctx.meta.$responseHeaders = {
-				"X-Custom-Header": "Custom content"
+				"X-Custom-Header": "Custom content",
+				"x-invalid-header": "\r\nBOOM"
 			};
 
 			throw new MoleculerServerError("It is a wrong action! I always throw error!");
 		},
 
+		invalidResponseHeaders(ctx) {
+			ctx.meta.$responseHeaders = {
+				"x-valid-header": "valid header",
+				"x-invalid-header": "\r\nBOOM"
+			};
+
+			return {
+				"x-invalid-header": "\r\nBOOM"
+			};
+		},
+
 		update: {
 			rest: ["PUT /update", "PATCH /update"],
 			params: {