Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🐛 Fix XSS Vulnerability in SVG Upload Handling #19646

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+28 −34
Open

🐛 Fix XSS Vulnerability in SVG Upload Handling #19646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b7dfa0
Integrate DOMPurify for SVG file sanitization to prevent XSS attacks
TeneBrae93 Feb 2, 2024
733de57
Implement XSS protection for SVG uploads in update.js
TeneBrae93 Feb 2, 2024
731fc25
Update package.json
TeneBrae93 Feb 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 25 additions & 30 deletions ghost/core/core/server/web/api/middleware/upload.js
View file
Open in desktop
Expand Up @@ -6,6 +6,8 @@ const errors = require('@tryghost/errors');
const config = require('../../../../shared/config');
const tpl = require('@tryghost/tpl');
const logging = require('@tryghost/logging');
const { JSDOM } = require('jsdom');
const createDOMPurify = require('dompurify');

const messages = {
db: {
Expand All @@ -26,7 +28,7 @@ const messages = {
},
images: {
missingFile: 'Please select an image.',
invalidFile: 'Please select a valid image.'
invalidFile: 'Unsupported file type. SVG files must not contain malicious content.'
},
icons: {
missingFile: 'Please select an icon.',
Expand All @@ -49,7 +51,6 @@ const deleteSingleFile = file => fs.unlink(file.path).catch(err => logging.error

const single = name => function singleUploadFunction(req, res, next) {
const singleUpload = upload.single(name);

singleUpload(req, res, (err) => {
if (err) {
return next(err);
Expand All @@ -60,11 +61,11 @@ const single = name => function singleUploadFunction(req, res, next) {
res.removeListener('close', deleteFiles);
if (!req.disableUploadClear) {
if (req.files) {
return req.files.forEach(deleteSingleFile);
req.files.forEach(deleteSingleFile);
}

if (req.file) {
return deleteSingleFile(req.file);
deleteSingleFile(req.file);
}
}
};
Expand All @@ -85,22 +86,20 @@ const media = (fileName, thumbName) => function mediaUploadFunction(req, res, ne
name: thumbName,
maxCount: 1
}]);

mediaUpload(req, res, (err) => {
if (err) {
return next(err);
}

if (enabledClear) {
const deleteFiles = () => {
res.removeListener('finish', deleteFiles);
res.removeListener('close', deleteFiles);
if (!req.disableUploadClear) {
if (req.files.file) {
return req.files.file.forEach(deleteSingleFile);
req.files.file.forEach(deleteSingleFile);
}
if (req.files.thumbnail) {
return req.files.thumbnail.forEach(deleteSingleFile);
req.files.thumbnail.forEach(deleteSingleFile);
}
}
};
Expand All @@ -109,7 +108,6 @@ const media = (fileName, thumbName) => function mediaUploadFunction(req, res, ne
res.on('close', deleteFiles);
}
}

next();
});
};
Expand All @@ -119,23 +117,28 @@ const checkFileExists = (fileData) => {
};

const checkFileIsValid = (fileData, types, extensions) => {
const type = fileData.mimetype;
const fileType = fileData.mimetype;
const fileExt = path.extname(fileData.name).toLowerCase();

// Adjust the logic to sanitize SVG files and accept them if sanitized
if (fileExt === '.svg') {
const window = new JSDOM('').window;
const dompurify = createDOMPurify(window);
const content = fs.readFileSync(fileData.path, 'utf8');
const sanitizedContent = dompurify.sanitize(content, {RETURN_TRUSTED_TYPE: false});

fs.writeFileSync(fileData.path, sanitizedContent, 'utf8');

if (types.includes(type) && extensions.includes(fileData.ext)) {
// Always return true as the file is sanitized before being validated
return true;
} else if (!types.includes(fileType) || !extensions.includes(fileExt)) {
return false;
}

return false;
return true;
};

/**
*
* @param {Object} options
* @param {String} options.type - type of the file
* @returns {Function}
*/
const validation = function ({type}) {
// if we finish the data/importer logic, we forward the request to the specified importer
return function uploadValidation(req, res, next) {
const extensions = (config.get('uploads')[type] && config.get('uploads')[type].extensions) || [];
const contentTypes = (config.get('uploads')[type] && config.get('uploads')[type].contentTypes) || [];
Expand All @@ -144,7 +147,6 @@ const validation = function ({type}) {
req.file.name = req.file.originalname;
req.file.type = req.file.mimetype;

// Check if a file was provided
if (!checkFileExists(req.file)) {
return next(new errors.ValidationError({
message: tpl(messages[type].missingFile)
Expand All @@ -153,23 +155,16 @@ const validation = function ({type}) {

req.file.ext = path.extname(req.file.name).toLowerCase();

// Check if the file is valid
if (!checkFileIsValid(req.file, contentTypes, extensions)) {
return next(new errors.UnsupportedMediaTypeError({
message: tpl(messages[type].invalidFile, {extensions: extensions})
message: tpl(messages[type].invalidFile, {extensions: extensions.join(', ')})
}));
}

next();
};
};

/**
*
* @param {Object} options
* @param {String} options.type - type of the file
* @returns {Function}
*/
const mediaValidation = function ({type}) {
return function mediaUploadValidation(req, res, next) {
const extensions = (config.get('uploads')[type] && config.get('uploads')[type].extensions) || [];
Expand All @@ -192,7 +187,7 @@ const mediaValidation = function ({type}) {

if (!checkFileIsValid(req.file, contentTypes, extensions)) {
return next(new errors.UnsupportedMediaTypeError({
message: tpl(messages[type].invalidFile, {extensions: extensions})
message: tpl(messages[type].invalidFile, {extensions: extensions.join(', ')})
}));
}

Expand All @@ -212,7 +207,7 @@ const mediaValidation = function ({type}) {

if (!checkFileIsValid(req.thumbnail, thumbnailContentTypes, thumbnailExtensions)) {
return next(new errors.UnsupportedMediaTypeError({
message: tpl(messages.thumbnail.invalidFile, {extensions: thumbnailExtensions})
message: tpl(messages.thumbnail.invalidFile, {extensions: thumbnailExtensions.join(', ')})
}));
}
}
Expand Down
7 changes: 3 additions & 4 deletions ghost/core/package.json
View file
Open in desktop
Expand Up @@ -192,9 +192,6 @@
"intl": "1.2.5",
"intl-messageformat": "5.4.3",
"js-yaml": "4.1.0",
"jsdom": "22.1.0",
"json-stable-stringify": "1.1.1",
"jsonpath": "1.1.1",
"jsonwebtoken": "8.5.1",
"juice": "9.1.0",
"keypair": "1.0.4",
Expand All @@ -221,7 +218,9 @@
"ws": "8.16.0",
"xml": "1.0.1",
"y-protocols": "1.0.6",
"yjs": "13.6.11"
"yjs": "13.6.11",
"dompurify": "^2.3.4",
"jsdom": "^19.0.0"
},
"optionalDependencies": {
"@sentry/profiling-node": "1.3.5",
Expand Down