fix(security): chunked-upload init checks the size cap before the type allow-list
Keeps the size error first, as before the allow-list landed, and pins the allow-list gate in the size-limit suite: a .html filename is refused whatever MIME the client declares.
This commit is contained in:
@@ -63,10 +63,10 @@ describe('admin upload per-file size limit (general_max_file_size_mb)', () => {
|
||||
.set('Authorization', `Bearer ${adminToken}`)
|
||||
.attach('photos', Buffer.alloc(bytes, 0x41), { filename, contentType: 'image/jpeg' });
|
||||
|
||||
const postChunkedInit = (fileSize) => request(app)
|
||||
const postChunkedInit = (fileSize, filename = 'clip.mp4') => request(app)
|
||||
.post(`/api/admin/photos/${eventId}/chunked-upload/init`)
|
||||
.set('Authorization', `Bearer ${adminToken}`)
|
||||
.send({ filename: 'clip.mp4', fileSize, mimeType: 'video/mp4', totalChunks: 1 });
|
||||
.send({ filename, fileSize, mimeType: 'video/mp4', totalChunks: 1 });
|
||||
|
||||
beforeAll(async () => {
|
||||
({ db, cleanup } = await bootCrmDb());
|
||||
@@ -108,6 +108,20 @@ describe('admin upload per-file size limit (general_max_file_size_mb)', () => {
|
||||
|
||||
uploadSettings = require('../../src/services/uploadSettings');
|
||||
|
||||
// chunked-upload/init now enforces the admin allow-list on the filename
|
||||
// extension (the declared mimeType is ignored), exactly like the
|
||||
// multipart path; the default list is images only, so admit mp4 here.
|
||||
await db('app_settings')
|
||||
.insert({
|
||||
setting_key: 'general_allowed_file_types',
|
||||
setting_value: JSON.stringify('jpg,jpeg,png,webp,mp4'),
|
||||
setting_type: 'general',
|
||||
updated_at: new Date().toISOString(),
|
||||
})
|
||||
.onConflict('setting_key')
|
||||
.merge({ setting_value: JSON.stringify('jpg,jpeg,png,webp,mp4') });
|
||||
uploadSettings.clearAllowedTypesCache();
|
||||
|
||||
app = express();
|
||||
app.use(express.json());
|
||||
app.use('/api/admin/photos', require('../../src/routes/adminPhotos'));
|
||||
@@ -129,6 +143,13 @@ describe('admin upload per-file size limit (general_max_file_size_mb)', () => {
|
||||
expect(res.body.error).toBe('File too large. Maximum size is 1 MB per file.');
|
||||
});
|
||||
|
||||
it('rejects a chunked upload whose extension is not on the allow-list, whatever MIME it declares', async () => {
|
||||
await setLimitMb(50);
|
||||
const res = await postChunkedInit(1024, 'page.html');
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toBe('File type not allowed');
|
||||
});
|
||||
|
||||
it('rejects chunk bytes over the limit regardless of the declared fileSize', async () => {
|
||||
await setLimitMb(1);
|
||||
const initRes = await postChunkedInit(1);
|
||||
|
||||
@@ -1626,20 +1626,6 @@ router.post('/:eventId/chunked-upload/init', adminAuth, requirePermission('photo
|
||||
return res.status(400).json({ error: 'Missing required fields: filename, fileSize' });
|
||||
}
|
||||
|
||||
// The client-declared mimeType is not trusted. It used to be stored on
|
||||
// the photo row verbatim and echoed as Content-Type by the gallery
|
||||
// routes, so a JPEG/HTML polyglot declared as text/html rendered inline
|
||||
// on the app origin. The MIME is derived from the extension instead,
|
||||
// and the extension has to be on the admin's allow-list, which is what
|
||||
// the multipart path enforces through its multer fileFilter.
|
||||
const ext = path.extname(String(filename)).slice(1).toLowerCase();
|
||||
const mimeType = Object.prototype.hasOwnProperty.call(EXTENSION_TO_MIME, ext)
|
||||
? EXTENSION_TO_MIME[ext]
|
||||
: null;
|
||||
const allowedMimeTypes = await getAllowedMimeTypes();
|
||||
if (!mimeType || !allowedMimeTypes.includes(mimeType)) {
|
||||
return res.status(400).json({ error: 'File type not allowed' });
|
||||
}
|
||||
|
||||
// Validate file size against the configured per-file cap. Hardcoding 10GB
|
||||
// here let the chunked path sidestep general_max_file_size_mb entirely.
|
||||
@@ -1655,6 +1641,21 @@ router.post('/:eventId/chunked-upload/init', adminAuth, requirePermission('photo
|
||||
});
|
||||
}
|
||||
|
||||
// The client-declared mimeType is not trusted. It used to be stored on
|
||||
// the photo row verbatim and echoed as Content-Type by the gallery
|
||||
// routes, so a JPEG/HTML polyglot declared as text/html rendered inline
|
||||
// on the app origin. The MIME is derived from the extension instead,
|
||||
// and the extension has to be on the admin's allow-list, which is what
|
||||
// the multipart path enforces through its multer fileFilter.
|
||||
const ext = path.extname(String(filename)).slice(1).toLowerCase();
|
||||
const mimeType = Object.prototype.hasOwnProperty.call(EXTENSION_TO_MIME, ext)
|
||||
? EXTENSION_TO_MIME[ext]
|
||||
: null;
|
||||
const allowedMimeTypes = await getAllowedMimeTypes();
|
||||
if (!mimeType || !allowedMimeTypes.includes(mimeType)) {
|
||||
return res.status(400).json({ error: 'File type not allowed' });
|
||||
}
|
||||
|
||||
const result = await chunkedUpload.initializeUpload({
|
||||
filename,
|
||||
fileSize,
|
||||
|
||||
Reference in New Issue
Block a user