fix(api): sanitize incoming user session id's

pujitm · pujitm · commit f5e3424b7970 · 2025-01-13T11:50:24.000-05:00
diff --git a/api/src/unraid-api/auth/cookie.service.spec.ts b/api/src/unraid-api/auth/cookie.service.spec.ts
@@ -46,15 +46,17 @@ describe.concurrent('CookieService', () => {
     it('handles session names robustly', ({ expect }) => {
         const session = (name?: unknown) => service.getSessionFilePath(name as string);
         expect(session('foo')).toEqual('/tmp/php/sessions/sess_foo');
+        expect(session('foo123')).toEqual('/tmp/php/sessions/sess_foo123');
+        expect(session('/foo123*&/^\n\r\'"!;:/../~`+=@#$%(?) \t/~/.profile')).toEqual('/tmp/php/sessions/sess_foo123profile');
         expect(session('')).toEqual('/tmp/php/sessions/sess_');
-        expect(session(null)).toEqual('/tmp/php/sessions/sess_null');
-        expect(session(undefined)).toEqual('/tmp/php/sessions/sess_undefined');
-        expect(session(1)).toEqual('/tmp/php/sessions/sess_1');
-        expect(session(1.0)).toEqual('/tmp/php/sessions/sess_1');
-        expect(session(1.1)).toEqual('/tmp/php/sessions/sess_1.1');
-        expect(session({})).toEqual('/tmp/php/sessions/sess_[object Object]');
-        expect(session(['foo', 'bar'])).toEqual('/tmp/php/sessions/sess_foo,bar');
-        expect(session('foo/bar')).toEqual('/tmp/php/sessions/sess_foo/bar');
+        expect(session(null)).toEqual('/tmp/php/sessions/sess_');
+        expect(session(undefined)).toEqual('/tmp/php/sessions/sess_');
+        expect(session(1)).toEqual('/tmp/php/sessions/sess_');
+        expect(session(1.0)).toEqual('/tmp/php/sessions/sess_');
+        expect(session(1.1)).toEqual('/tmp/php/sessions/sess_');
+        expect(session({})).toEqual('/tmp/php/sessions/sess_');
+        expect(session(['foo', 'bar'])).toEqual('/tmp/php/sessions/sess_');
+        expect(session('foo/bar')).toEqual('/tmp/php/sessions/sess_foobar');
     });
 
     it('can read an existing session & reject a non-existent one', async ({ expect }) => {
diff --git a/api/src/unraid-api/auth/cookie.service.ts b/api/src/unraid-api/auth/cookie.service.ts
@@ -83,6 +83,12 @@ export class CookieService {
      * @returns the full path to the session file on disk.
      */
     public getSessionFilePath(sessionId: string): string {
-        return join(this.opts.sessionDir, `sess_${sessionId}`);
+        if (typeof sessionId !== 'string') {
+            return join(this.opts.sessionDir, `sess_`);
+        }
+        // sanitize incoming session id to prevent e.g. directory traversal attacks
+        // only allow alpha-numeric characters
+        const sanitizedSessionId = sessionId.replace(/[^a-zA-Z0-9]/g, '');
+        return join(this.opts.sessionDir, `sess_${sanitizedSessionId}`);
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -83,6 +83,12 @@ export class CookieService {`
`83`	`83`	`* @returns the full path to the session file on disk.`
`84`	`84`	`*/`
`85`	`85`	`public getSessionFilePath(sessionId: string): string {`
`86`		- return join(this.opts.sessionDir, `sess_${sessionId}`);
	`86`	`+ if (typeof sessionId !== 'string') {`
	`87`	+ return join(this.opts.sessionDir, `sess_`);
	`88`	`+ }`
	`89`	`+ // sanitize incoming session id to prevent e.g. directory traversal attacks`
	`90`	`+ // only allow alpha-numeric characters`
	`91`	`+ const sanitizedSessionId = sessionId.replace(/[^a-zA-Z0-9]/g, '');`
	`92`	+ return join(this.opts.sessionDir, `sess_${sanitizedSessionId}`);
`87`	`93`	`}`
`88`	`94`	`}`