fix: svg+xml file detection (#13276)

Adds logic for svg+xml file type detection.

---------

Co-authored-by: Philipp Schneider <47689073+philipp-tailor@users.noreply.github.com>
This commit is contained in:
Jarrod Flesch
2025-07-25 14:33:53 -04:00
committed by GitHub
parent 23bd67515c
commit e8f6cb5ed1
7 changed files with 109 additions and 14 deletions

View File

@@ -4,6 +4,7 @@ import type { checkFileRestrictionsParams, FileAllowList } from './types.js'
import { ValidationError } from '../errors/index.js' import { ValidationError } from '../errors/index.js'
import { validateMimeType } from '../utilities/validateMimeType.js' import { validateMimeType } from '../utilities/validateMimeType.js'
import { detectSvgFromXml } from './detectSvgFromXml.js'
/** /**
* Restricted file types and their extensions. * Restricted file types and their extensions.
@@ -69,7 +70,17 @@ export const checkFileRestrictions = async ({
// Secondary mimetype check to assess file type from buffer // Secondary mimetype check to assess file type from buffer
if (configMimeTypes.length > 0) { if (configMimeTypes.length > 0) {
const detected = await fileTypeFromBuffer(file.data) let detected = await fileTypeFromBuffer(file.data)
// Handle SVG files that are detected as XML due to <?xml declarations
if (
detected?.mime === 'application/xml' &&
configMimeTypes.some((type) => type.includes('svg')) &&
detectSvgFromXml(file.data)
) {
detected = { ext: 'svg' as any, mime: 'image/svg+xml' as any }
}
const passesMimeTypeCheck = detected?.mime && validateMimeType(detected.mime, configMimeTypes) const passesMimeTypeCheck = detected?.mime && validateMimeType(detected.mime, configMimeTypes)
if (detected && !passesMimeTypeCheck) { if (detected && !passesMimeTypeCheck) {

View File

@@ -0,0 +1,49 @@
/**
* Securely detect if an XML buffer contains a valid SVG document
*/
export function detectSvgFromXml(buffer: Buffer): boolean {
try {
// Limit buffer size to prevent processing large malicious files
const maxSize = 2048
const content = buffer.toString('utf8', 0, Math.min(buffer.length, maxSize))
// Check for XML declaration and extract encoding if present
const xmlDeclMatch = content.match(/^<\?xml[^>]*encoding=["']([^"']+)["']/i)
const declaredEncoding = xmlDeclMatch?.[1]?.toLowerCase()
// Only support safe encodings
if (declaredEncoding && !['ascii', 'utf-8', 'utf8'].includes(declaredEncoding)) {
return false
}
// Remove XML declarations, comments, and processing instructions
const cleanContent = content
.replace(/<\?xml[^>]*\?>/gi, '')
.replace(/<!--[\s\S]*?-->/g, '')
.replace(/<\?[^>]*\?>/g, '')
.trim()
// Find the first actual element (root element)
const rootElementMatch = cleanContent.match(/^<(\w+)(?:\s|>)/)
if (!rootElementMatch || rootElementMatch[1] !== 'svg') {
return false
}
// Validate SVG namespace - must be present for valid SVG
const svgNamespaceRegex = /xmlns=["']http:\/\/www\.w3\.org\/2000\/svg["']/
if (!svgNamespaceRegex.test(content)) {
return false
}
// Additional validation: ensure it's not malformed
const svgOpenTag = content.match(/<svg[\s>]/)
if (!svgOpenTag) {
return false
}
return true
} catch (_error) {
// If any error occurs during parsing, treat as not SVG
return false
}
}

View File

@@ -84,7 +84,7 @@ export interface Config {
'payload-migrations': PayloadMigrationsSelect<false> | PayloadMigrationsSelect<true>; 'payload-migrations': PayloadMigrationsSelect<false> | PayloadMigrationsSelect<true>;
}; };
db: { db: {
defaultIDType: string; defaultIDType: number;
}; };
globals: { globals: {
menu: Menu; menu: Menu;
@@ -124,7 +124,7 @@ export interface UserAuthOperations {
* via the `definition` "posts". * via the `definition` "posts".
*/ */
export interface Post { export interface Post {
id: string; id: number;
title?: string | null; title?: string | null;
content?: { content?: {
root: { root: {
@@ -149,7 +149,7 @@ export interface Post {
* via the `definition` "media". * via the `definition` "media".
*/ */
export interface Media { export interface Media {
id: string; id: number;
updatedAt: string; updatedAt: string;
createdAt: string; createdAt: string;
url?: string | null; url?: string | null;
@@ -193,7 +193,7 @@ export interface Media {
* via the `definition` "users". * via the `definition` "users".
*/ */
export interface User { export interface User {
id: string; id: number;
updatedAt: string; updatedAt: string;
createdAt: string; createdAt: string;
email: string; email: string;
@@ -217,24 +217,24 @@ export interface User {
* via the `definition` "payload-locked-documents". * via the `definition` "payload-locked-documents".
*/ */
export interface PayloadLockedDocument { export interface PayloadLockedDocument {
id: string; id: number;
document?: document?:
| ({ | ({
relationTo: 'posts'; relationTo: 'posts';
value: string | Post; value: number | Post;
} | null) } | null)
| ({ | ({
relationTo: 'media'; relationTo: 'media';
value: string | Media; value: number | Media;
} | null) } | null)
| ({ | ({
relationTo: 'users'; relationTo: 'users';
value: string | User; value: number | User;
} | null); } | null);
globalSlug?: string | null; globalSlug?: string | null;
user: { user: {
relationTo: 'users'; relationTo: 'users';
value: string | User; value: number | User;
}; };
updatedAt: string; updatedAt: string;
createdAt: string; createdAt: string;
@@ -244,10 +244,10 @@ export interface PayloadLockedDocument {
* via the `definition` "payload-preferences". * via the `definition` "payload-preferences".
*/ */
export interface PayloadPreference { export interface PayloadPreference {
id: string; id: number;
user: { user: {
relationTo: 'users'; relationTo: 'users';
value: string | User; value: number | User;
}; };
key?: string | null; key?: string | null;
value?: value?:
@@ -267,7 +267,7 @@ export interface PayloadPreference {
* via the `definition` "payload-migrations". * via the `definition` "payload-migrations".
*/ */
export interface PayloadMigration { export interface PayloadMigration {
id: string; id: number;
name?: string | null; name?: string | null;
batch?: number | null; batch?: number | null;
updatedAt: string; updatedAt: string;
@@ -393,7 +393,7 @@ export interface PayloadMigrationsSelect<T extends boolean = true> {
* via the `definition` "menu". * via the `definition` "menu".
*/ */
export interface Menu { export interface Menu {
id: string; id: number;
globalText?: string | null; globalText?: string | null;
updatedAt?: string | null; updatedAt?: string | null;
createdAt?: string | null; createdAt?: string | null;

View File

@@ -40,6 +40,7 @@ import {
restrictFileTypesSlug, restrictFileTypesSlug,
skipAllowListSafeFetchMediaSlug, skipAllowListSafeFetchMediaSlug,
skipSafeFetchMediaSlug, skipSafeFetchMediaSlug,
svgOnlySlug,
threeDimensionalSlug, threeDimensionalSlug,
unstoredMediaSlug, unstoredMediaSlug,
versionSlug, versionSlug,
@@ -910,6 +911,14 @@ export default buildConfigWithDefaults({
BulkUploadsCollection, BulkUploadsCollection,
SimpleRelationshipCollection, SimpleRelationshipCollection,
FileMimeType, FileMimeType,
{
slug: svgOnlySlug,
fields: [],
upload: {
mimeTypes: ['image/svg+xml'],
staticDir: path.resolve(dirname, './svg-only'),
},
},
], ],
onInit: async (payload) => { onInit: async (payload) => {
const uploadsDir = path.resolve(dirname, './media') const uploadsDir = path.resolve(dirname, './media')

View File

@@ -25,6 +25,7 @@ import {
restrictFileTypesSlug, restrictFileTypesSlug,
skipAllowListSafeFetchMediaSlug, skipAllowListSafeFetchMediaSlug,
skipSafeFetchMediaSlug, skipSafeFetchMediaSlug,
svgOnlySlug,
unstoredMediaSlug, unstoredMediaSlug,
usersSlug, usersSlug,
} from './shared.js' } from './shared.js'
@@ -370,6 +371,21 @@ describe('Collections - Uploads', () => {
}) })
describe('Local API', () => { describe('Local API', () => {
describe('create', () => {
it('should create documents when passing filePath', async () => {
const expectedPath = path.join(dirname, './svg-only')
const svgFilePath = path.resolve(dirname, './svgWithXml.svg')
const doc = await payload.create({
collection: svgOnlySlug as CollectionSlug,
data: {},
filePath: svgFilePath,
})
expect(await fileExists(path.join(expectedPath, doc.filename))).toBe(true)
})
})
describe('update', () => { describe('update', () => {
it('should remove existing media on re-upload - by ID', async () => { it('should remove existing media on re-upload - by ID', async () => {
// Create temp file // Create temp file

View File

@@ -37,3 +37,4 @@ export const constructorOptionsSlug = 'constructor-options'
export const bulkUploadsSlug = 'bulk-uploads' export const bulkUploadsSlug = 'bulk-uploads'
export const fileMimeTypeSlug = 'file-mime-type' export const fileMimeTypeSlug = 'file-mime-type'
export const svgOnlySlug = 'svg-only'

View File

@@ -0,0 +1,9 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg xmlns="http://www.w3.org/2000/svg"
width="1"
height="1">
<rect
width="1"
height="1"
style="fill:#666;" />
</svg>

After

Width:  |  Height:  |  Size: 204 B