fix: [#1585] Fixes a security vulnerability that allowed for server s…

…ide code to be executed by a <script> tag (#1588)
capricorn86 · Nov 6, 2024 · d23834c · d23834c
1 parent 5ee0b16
commit d23834c
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 7 deletions.
diff --git a/packages/happy-dom/src/fetch/utilities/SyncFetchScriptBuilder.ts b/packages/happy-dom/src/fetch/utilities/SyncFetchScriptBuilder.ts
@@ -44,7 +44,9 @@ export default class SyncFetchScriptBuilder {
 									null,
 									4
 								)};
-                const request = sendRequest(\`${request.url.href}\`, options, (incomingMessage) => {
+                const request = sendRequest(${JSON.stringify(
+									request.url.href
+								)}, options, (incomingMessage) => {
                     let data = Buffer.alloc(0);
                     incomingMessage.on('data', (chunk) => {
                         data = Buffer.concat([data, Buffer.from(chunk)]);

diff --git a/packages/happy-dom/test/fetch/SyncFetch.test.ts b/packages/happy-dom/test/fetch/SyncFetch.test.ts
@@ -252,8 +252,7 @@ describe('SyncFetch', () => {
 		it('Should not allow to inject code into scripts executed using child_process.execFileSync().', () => {
 			browserFrame.url = 'https://localhost:8080/';
 
-			const url =
-				"https://localhost:8080/`+require('child_process').execSync('id')+`/'+require('child_process').execSync('id')+'";
+			const url = `https://localhost:8080/\`+require('child_process').execSync('id')+\`/'+require('child_process').execSync('id')+'/?key="+require('child_process').execSync('id')+"`;
 			const responseText = 'test';
 
 			mockModule('child_process', {
@@ -267,7 +266,7 @@ describe('SyncFetch', () => {
 					expect(args[1]).toBe(
 						SyncFetchScriptBuilder.getScript({
 							url: new URL(
-								"https://localhost:8080/%60+require('child_process').execSync('id')+%60/'+require('child_process').execSync('id')+'"
+								`https://localhost:8080/\`+require('child_process').execSync('id')+\`/'+require('child_process').execSync('id')+'/?key="+require('child_process').execSync('id')+"`
 							),
 							method: 'GET',
 							headers: {
@@ -280,11 +279,9 @@ describe('SyncFetch', () => {
 							body: null
 						})
 					);
-					// new URL() will convert ` into %60
-					// By using ` for the URL string within the script, we can prevent the script from being injected
 					expect(
 						args[1].includes(
-							`\`https://localhost:8080/%60+require('child_process').execSync('id')+%60/'+require('child_process').execSync('id')+'\``
+							`"https://localhost:8080/%60+require('child_process').execSync('id')+%60/'+require('child_process').execSync('id')+'/?key=%22+require(%27child_process%27).execSync(%27id%27)+%22"`
 						)
 					).toBe(true);
 					expect(options).toEqual({