Skip to content

Commit 9e99279

Browse files
authored
fix: Stored XSS via trailing-dot filename bypassing file upload extension blocklist ([GHSA-7wqv-xjf3-x35v](GHSA-7wqv-xjf3-x35v)) (#10490)
1 parent 4a7fd10 commit 9e99279

5 files changed

Lines changed: 219 additions & 10 deletions

File tree

spec/Utils.spec.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,4 +212,25 @@ describe('Utils', () => {
212212
expect(error.message).toBe('Detailed error message');
213213
});
214214
});
215+
216+
describe('getFileExtension', () => {
217+
const cases = [
218+
['file.txt', 'txt'],
219+
['file.tar.gz', 'gz'],
220+
['.hidden', 'hidden'],
221+
['file.', ''],
222+
['file..', ''],
223+
['file', ''],
224+
['', ''],
225+
[null, ''],
226+
[undefined, ''],
227+
['poc.svg.', ''],
228+
['archive.tar.gz.', ''],
229+
];
230+
for (const [input, expected] of cases) {
231+
it(`returns ${JSON.stringify(expected)} for ${JSON.stringify(input)}`, () => {
232+
expect(Utils.getFileExtension(input)).toBe(expected);
233+
});
234+
}
235+
});
215236
});

spec/vulnerabilities.spec.js

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2832,6 +2832,170 @@ describe('Vulnerabilities', () => {
28322832
});
28332833
});
28342834

2835+
describe('(GHSA-7wqv-xjf3-x35v) Stored XSS via trailing-dot filename bypassing file extension blocklist', () => {
2836+
const headers = {
2837+
'X-Parse-Application-Id': 'test',
2838+
'X-Parse-REST-API-Key': 'rest',
2839+
};
2840+
2841+
beforeEach(async () => {
2842+
await reconfigureServer({
2843+
fileUpload: {
2844+
enableForPublic: true,
2845+
},
2846+
});
2847+
});
2848+
2849+
it('blocks trailing-dot SVG filename with dangerous _ContentType on JSON-body upload', async () => {
2850+
const svgContent = Buffer.from(
2851+
'<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>'
2852+
).toString('base64');
2853+
await expectAsync(
2854+
request({
2855+
method: 'POST',
2856+
url: 'http://localhost:8378/1/files/poc.svg.',
2857+
body: JSON.stringify({
2858+
_ApplicationId: 'test',
2859+
_JavaScriptKey: 'test',
2860+
_ContentType: 'image/svg+xml',
2861+
base64: svgContent,
2862+
}),
2863+
}).catch(e => {
2864+
throw new Error(e.data.error);
2865+
})
2866+
).toBeRejectedWith(jasmine.objectContaining({
2867+
message: jasmine.stringMatching(/File upload of extension .+ is disabled/),
2868+
}));
2869+
});
2870+
2871+
it('blocks trailing-dot SVG filename with dangerous Content-Type on binary upload', async () => {
2872+
await expectAsync(
2873+
request({
2874+
method: 'POST',
2875+
headers: {
2876+
...headers,
2877+
'Content-Type': 'image/svg+xml',
2878+
},
2879+
url: 'http://localhost:8378/1/files/poc.svg.',
2880+
body: '<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>',
2881+
}).catch(e => {
2882+
throw new Error(e.data.error);
2883+
})
2884+
).toBeRejectedWith(jasmine.objectContaining({
2885+
message: jasmine.stringMatching(/File upload of extension .+ is disabled/),
2886+
}));
2887+
});
2888+
2889+
it('blocks filename with mixed trailing dots and whitespace', async () => {
2890+
for (const filename of ['poc.svg..', 'poc.svg. ', 'poc.svg . ']) {
2891+
await expectAsync(
2892+
request({
2893+
method: 'POST',
2894+
headers: {
2895+
...headers,
2896+
'Content-Type': 'image/svg+xml',
2897+
},
2898+
url: `http://localhost:8378/1/files/${encodeURIComponent(filename)}`,
2899+
body: '<svg/>',
2900+
}).catch(e => {
2901+
throw new Error(e.data.error);
2902+
})
2903+
).toBeRejectedWith(jasmine.objectContaining({
2904+
message: jasmine.stringMatching(/File upload of extension .+ is disabled/),
2905+
}));
2906+
}
2907+
});
2908+
2909+
it('still allows trailing-dot filename with allowed Content-Type', async () => {
2910+
const adapter = Config.get('test').filesController.adapter;
2911+
const spy = spyOn(adapter, 'createFile').and.callThrough();
2912+
const response = await request({
2913+
method: 'POST',
2914+
url: 'http://localhost:8378/1/files/notes.txt.',
2915+
body: JSON.stringify({
2916+
_ApplicationId: 'test',
2917+
_JavaScriptKey: 'test',
2918+
_ContentType: 'text/plain',
2919+
base64: Buffer.from('hello').toString('base64'),
2920+
}),
2921+
headers,
2922+
});
2923+
expect(response.status).toBe(201);
2924+
expect(spy).toHaveBeenCalled();
2925+
});
2926+
2927+
it('FilesController treats trailing-dot filename as extensionless when appending derived extension via master key upload', async () => {
2928+
await reconfigureServer({
2929+
fileUpload: {
2930+
enableForPublic: true,
2931+
},
2932+
preserveFileName: true,
2933+
});
2934+
const adapter = Config.get('test').filesController.adapter;
2935+
const spy = spyOn(adapter, 'createFile').and.callThrough();
2936+
const response = await request({
2937+
method: 'POST',
2938+
url: 'http://localhost:8378/1/files/poc.svg.',
2939+
headers: {
2940+
'X-Parse-Application-Id': 'test',
2941+
'X-Parse-Master-Key': 'test',
2942+
'Content-Type': 'image/svg+xml',
2943+
},
2944+
body: '<svg/>',
2945+
});
2946+
expect(response.status).toBe(201);
2947+
expect(spy).toHaveBeenCalled();
2948+
const filenameArg = spy.calls.mostRecent().args[0];
2949+
const contentTypeArg = spy.calls.mostRecent().args[2];
2950+
expect(filenameArg).toBe('poc.svg.svg');
2951+
expect(contentTypeArg).toBe('image/svg+xml');
2952+
});
2953+
2954+
it('allows trailing-dot filename when no Content-Type is supplied (no XSS path)', async () => {
2955+
// Trailing-dot filename with no caller-supplied Content-Type: the
2956+
// blocklist gate skips because no extension can be determined, but no
2957+
// attacker-controlled Content-Type reaches the storage adapter — only
2958+
// the SDK's benign default — so no stored XSS is possible.
2959+
const adapter = Config.get('test').filesController.adapter;
2960+
const spy = spyOn(adapter, 'createFile').and.callThrough();
2961+
const response = await request({
2962+
method: 'POST',
2963+
headers: {
2964+
'X-Parse-Application-Id': 'test',
2965+
'X-Parse-REST-API-Key': 'rest',
2966+
},
2967+
url: 'http://localhost:8378/1/files/poc.svg.',
2968+
body: '<svg/>',
2969+
});
2970+
expect(response.status).toBe(201);
2971+
expect(spy).toHaveBeenCalled();
2972+
const contentTypeArg = spy.calls.mostRecent().args[2];
2973+
expect(contentTypeArg).not.toMatch(/svg|html|xml|xhtml|xslt|mathml/i);
2974+
});
2975+
2976+
it('falls back to raw Content-Type when Content-Type is malformed (no slash)', async () => {
2977+
// Exercises the last-resort branch: when both the filename has no usable
2978+
// extension AND the Content-Type lacks a "/" subtype to parse, the raw
2979+
// Content-Type is used as the extension so a malformed header that
2980+
// matches a blocked pattern still trips the blocklist.
2981+
await expectAsync(
2982+
request({
2983+
method: 'POST',
2984+
headers: {
2985+
...headers,
2986+
'Content-Type': 'svg',
2987+
},
2988+
url: 'http://localhost:8378/1/files/poc',
2989+
body: '<svg/>',
2990+
}).catch(e => {
2991+
throw new Error(e.data.error);
2992+
})
2993+
).toBeRejectedWith(jasmine.objectContaining({
2994+
message: jasmine.stringMatching(/File upload of extension svg is disabled/),
2995+
}));
2996+
});
2997+
});
2998+
28352999
describe('(GHSA-9ccr-fpp6-78qf) Schema poisoning via __proto__ bypassing requestKeywordDenylist and addField CLP', () => {
28363000
const headers = {
28373001
'Content-Type': 'application/json',

src/Controllers/FilesController.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
import { randomHexString } from '../cryptoUtils';
33
import AdaptableController from './AdaptableController';
44
import { validateFilename, FilesAdapter } from '../Adapters/Files/FilesAdapter';
5-
import path from 'path';
65
const Parse = require('parse').Parse;
6+
const Utils = require('../Utils');
77

88
const legacyFilesRegex = new RegExp(
99
'^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}-.*'
@@ -15,12 +15,13 @@ export class FilesController extends AdaptableController {
1515
}
1616

1717
async createFile(config, filename, data, contentType, options) {
18-
const extname = path.extname(filename);
19-
18+
const extname = Utils.getFileExtension(filename);
2019
const hasExtension = extname.length > 0;
2120
const mime = (await import('mime')).default
2221
if (!hasExtension && contentType && mime.getExtension(contentType)) {
23-
filename = filename + '.' + mime.getExtension(contentType);
22+
// Avoid producing a doubled dot when the filename already ends in one
23+
const separator = filename.endsWith('.') ? '' : '.';
24+
filename = filename + separator + mime.getExtension(contentType);
2425
} else if (hasExtension) {
2526
contentType = mime.getType(filename) || contentType;
2627
}

src/Routers/FilesRouter.js

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -197,14 +197,20 @@ export class FilesRouter {
197197
}
198198
});
199199
};
200-
let extension = contentType;
201-
if (filename && filename.includes('.')) {
202-
extension = filename.substring(filename.lastIndexOf('.') + 1);
203-
} else if (contentType && contentType.includes('/')) {
204-
extension = contentType.split('/')[1];
205-
}
200+
let extension = Utils.getFileExtension(filename);
206201
// Strip MIME parameters (e.g. ";charset=utf-8") and whitespace
207202
extension = extension?.split(';')[0]?.replace(/\s+/g, '');
203+
// If the filename has no usable extension (no dot, trailing dot, or
204+
// whitespace-only suffix), fall back to the Content-Type subtype — same
205+
// as a dotless filename.
206+
if (!extension && contentType && contentType.includes('/')) {
207+
extension = contentType.split('/')[1]?.split(';')[0]?.replace(/\s+/g, '');
208+
}
209+
// Last resort for malformed inputs (e.g. Content-Type without a slash):
210+
// use the raw Content-Type so the existing rejection path still fires.
211+
if (!extension && contentType) {
212+
extension = contentType.split(';')[0]?.replace(/\s+/g, '');
213+
}
208214

209215
if (extension && !isValidExtension(extension)) {
210216
next(

src/Utils.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,23 @@ class Utils {
478478
}
479479
return current;
480480
}
481+
482+
/**
483+
* Returns the file extension as the substring after the last dot in the
484+
* filename. A trailing dot or a filename without a dot yields an empty
485+
* string. Callers apply any further normalization (whitespace, MIME
486+
* parameters, etc.) for their use case — this is a pure parser, not a
487+
* policy.
488+
*
489+
* @param {string} filename
490+
* @returns {string} the extension, or `''` if none
491+
*/
492+
static getFileExtension(filename) {
493+
if (!filename || !filename.includes('.')) {
494+
return '';
495+
}
496+
return filename.substring(filename.lastIndexOf('.') + 1);
497+
}
481498
}
482499

483500
module.exports = Utils;

0 commit comments

Comments
 (0)