Skip to content

Commit

Permalink
[mirotalksfu] - Improve security, add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
miroslavpejic85 committed Aug 15, 2024
1 parent 8d7901e commit 7287363
Show file tree
Hide file tree
Showing 6 changed files with 310 additions and 57 deletions.
2 changes: 1 addition & 1 deletion app/src/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ dependencies: {
* @license For commercial or closed source, contact us at [email protected] or purchase directly via CodeCanyon
* @license CodeCanyon: https://codecanyon.net/item/mirotalk-sfu-webrtc-realtime-video-conferences/40769970
* @author Miroslav Pejic - [email protected]
* @version 1.5.59
* @version 1.5.60
*
*/

Expand Down
126 changes: 78 additions & 48 deletions app/src/XSS.js
Original file line number Diff line number Diff line change
@@ -1,66 +1,96 @@
'use strict';

const xss = require('xss');
const { JSDOM } = require('jsdom');
const DOMPurify = require('dompurify');
const he = require('he');

// Initialize DOMPurify with jsdom
const window = new JSDOM('').window;
const purify = DOMPurify(window);

const Logger = require('./Logger');
const log = new Logger('Xss');

const checkXSS = (dataObject) => {
try {
if (Array.isArray(dataObject)) {
if (Object.keys(dataObject).length > 0 && typeof dataObject[0] === 'object') {
dataObject.forEach((obj) => {
for (const key in obj) {
if (obj.hasOwnProperty(key)) {
let objectJson = objectToJSONString(obj[key]);
if (objectJson) {
let jsonString = xss(objectJson);
let jsonObject = JSONStringToObject(jsonString);
if (jsonObject) {
obj[key] = jsonObject;
}
}
}
}
});
log.debug('XSS Array of Object sanitization done');
return dataObject;
// Configure DOMPurify
purify.setConfig({
ALLOWED_TAGS: ['a', 'img', 'div', 'span', 'svg', 'g', 'p'], // Allow specific tags
ALLOWED_ATTR: ['href', 'src', 'title', 'id', 'class'], // Allow specific attributes
ALLOWED_URI_REGEXP: /^(?!data:|javascript:|vbscript:|file:|view-source:).*/, // Disallow dangerous URIs
});

// Clean problematic attributes
function cleanAttributes(node) {
if (node.nodeType === window.Node.ELEMENT_NODE) {
// Remove dangerous attributes
const dangerousAttributes = ['onerror', 'onclick', 'onload', 'onmouseover', 'onfocus', 'onchange', 'oninput'];
dangerousAttributes.forEach((attr) => {
if (node.hasAttribute(attr)) {
node.removeAttribute(attr);
}
} else if (typeof dataObject === 'object') {
let objectJson = objectToJSONString(dataObject);
if (objectJson) {
let jsonString = xss(objectJson);
let jsonObject = JSONStringToObject(jsonString);
if (jsonObject) {
log.debug('XSS Object sanitization done');
return jsonObject;
}
});

// Handle special cases for 'data:' URIs
const src = node.getAttribute('src');
if (src && src.startsWith('data:')) {
node.removeAttribute('src');
}

// Remove unsafe 'style' attributes
if (node.hasAttribute('style')) {
const style = node.getAttribute('style');
if (style.includes('javascript:') || style.includes('data:')) {
node.removeAttribute('style');
}
} else if (typeof dataObject === 'string' || dataObject instanceof String) {
log.debug('XSS String sanitization done');
return xss(dataObject);
}
log.warn('XSS not sanitized', dataObject);
return dataObject;
} catch (error) {
log.error('XSS error', { data: dataObject, error: error });
return dataObject;
}
};

function objectToJSONString(dataObject) {
try {
return JSON.stringify(dataObject);
} catch (error) {
return false;
// Remove 'title' attribute if it contains dangerous content
if (node.hasAttribute('title')) {
const title = node.getAttribute('title');
if (title.includes('javascript:') || title.includes('data:') || title.includes('onerror')) {
node.removeAttribute('title');
}
}
}
}

function JSONStringToObject(jsonString) {
// Hook to clean specific attributes that can cause XSS
purify.addHook('beforeSanitizeAttributes', cleanAttributes);

// Main function to check and sanitize data
const checkXSS = (dataObject) => {
try {
return JSON.parse(jsonString);
return sanitizeData(dataObject);
} catch (error) {
return false;
log.error('Sanitization error:', error);
return dataObject; // Return original data in case of error
}
};

// Recursively sanitize data based on its type
function sanitizeData(data) {
if (typeof data === 'string') {
// Decode HTML entities and URL encoded content
const decodedData = he.decode(decodeURIComponent(data));
return purify.sanitize(decodedData);
}

if (Array.isArray(data)) {
return data.map(sanitizeData);
}

if (data && typeof data === 'object') {
return sanitizeObject(data);
}

return data; // For numbers, booleans, null, undefined
}

// Sanitize object properties
function sanitizeObject(obj) {
return Object.keys(obj).reduce((acc, key) => {
acc[key] = sanitizeData(obj[key]);
return acc;
}, {});
}

module.exports = checkXSS;
15 changes: 10 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "mirotalksfu",
"version": "1.5.59",
"version": "1.5.60",
"description": "WebRTC SFU browser-based video calls",
"main": "Server.js",
"scripts": {
Expand All @@ -27,7 +27,8 @@
"nms-start": "docker-compose -f rtmpServers/node-media-server/docker-compose.yml up -d",
"nms-stop": "docker-compose -f rtmpServers/node-media-server/docker-compose.yml down",
"nms-restart": "docker-compose -f rtmpServers/node-media-server/docker-compose.yml down && docker-compose -f rtmpServers/node-media-server/docker-compose.yml up -d",
"nms-logs": "docker logs -f mirotalk-nms"
"nms-logs": "docker logs -f mirotalk-nms",
"unit-test": "npx mocha tests/checkXSS.js"
},
"repository": {
"type": "git",
Expand Down Expand Up @@ -64,11 +65,14 @@
"compression": "1.7.4",
"cors": "2.8.5",
"crypto-js": "4.2.0",
"dompurify": "^3.1.6",
"express": "4.19.2",
"express-openid-connect": "^2.17.1",
"fluent-ffmpeg": "^2.1.3",
"he": "^1.2.0",
"httpolyglot": "0.1.2",
"js-yaml": "^4.1.0",
"jsdom": "^24.1.1",
"jsonwebtoken": "^9.0.2",
"mediasoup": "3.14.11",
"mediasoup-client": "3.7.16",
Expand All @@ -78,12 +82,13 @@
"qs": "6.13.0",
"socket.io": "4.7.5",
"swagger-ui-express": "5.0.1",
"uuid": "10.0.0",
"xss": "^1.0.15"
"uuid": "10.0.0"
},
"devDependencies": {
"mocha": "^10.7.3",
"node-fetch": "^3.3.2",
"nodemon": "^3.1.4",
"prettier": "3.3.3"
"prettier": "3.3.3",
"should": "^13.2.3"
}
}
4 changes: 2 additions & 2 deletions public/js/Room.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ if (location.href.substr(0, 5) !== 'https') location.href = 'https' + location.h
* @license For commercial or closed source, contact us at [email protected] or purchase directly via CodeCanyon
* @license CodeCanyon: https://codecanyon.net/item/mirotalk-sfu-webrtc-realtime-video-conferences/40769970
* @author Miroslav Pejic - [email protected]
* @version 1.5.59
* @version 1.5.60
*
*/

Expand Down Expand Up @@ -4444,7 +4444,7 @@ function showAbout() {
imageUrl: image.about,
customClass: { image: 'img-about' },
position: 'center',
title: 'WebRTC SFU v1.5.59',
title: 'WebRTC SFU v1.5.60',
html: `
<br />
<div id="about">
Expand Down
2 changes: 1 addition & 1 deletion public/js/RoomClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* @license For commercial or closed source, contact us at [email protected] or purchase directly via CodeCanyon
* @license CodeCanyon: https://codecanyon.net/item/mirotalk-sfu-webrtc-realtime-video-conferences/40769970
* @author Miroslav Pejic - [email protected]
* @version 1.5.59
* @version 1.5.60
*
*/

Expand Down
Loading

0 comments on commit 7287363

Please sign in to comment.