Skip to content

Commit

Permalink
[mirotalksfu] - fix open redirect
Browse files Browse the repository at this point in the history
  • Loading branch information
miroslavpejic85 committed Feb 11, 2025
1 parent 9b1d79c commit 775307d
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 15 deletions.
21 changes: 14 additions & 7 deletions app/src/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ dev 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.7.26
* @version 1.7.27
*
*/

Expand Down Expand Up @@ -408,20 +408,27 @@ function startServer() {

// Remove trailing slashes in url handle bad requests
app.use((err, req, res, next) => {
if (err instanceof SyntaxError || err.status === 400 || 'body' in err) {
if (err && (err instanceof SyntaxError || err.status === 400 || 'body' in err)) {
log.error('Request Error', {
header: req.headers,
body: req.body,
error: err.message,
});
return res.status(400).send({ status: 404, message: err.message }); // Bad request
}
if (req.path.substr(-1) === '/' && req.path.length > 1) {
let query = req.url.slice(req.path.length);
res.redirect(301, req.path.slice(0, -1) + query);
} else {
next();

// Prevent open redirect attacks by checking if the path is an external domain
const cleanPath = req.path.replace(/^\/+/, '');
if (/^([a-zA-Z0-9-]+\.)+[a-zA-Z]{2,}/.test(cleanPath)) {
return res.status(400).send('Bad Request: Potential Open Redirect Detected');
}

if (req.path.endsWith('/') && req.path.length > 1) {
let query = req.url.substring(req.path.length).replace(/\/$/, ''); // Ensure query params don't end in '/'
return res.redirect(301, req.path.slice(0, -1) + query);
}

next();
});

// OpenID Connect - Dynamically set baseURL based on incoming host and protocol
Expand Down
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "mirotalksfu",
"version": "1.7.26",
"version": "1.7.27",
"description": "WebRTC SFU browser-based video calls",
"main": "Server.js",
"scripts": {
Expand Down Expand Up @@ -58,13 +58,13 @@
},
"dependencies": {
"@mattermost/client": "10.2.0",
"@sentry/node": "^9.0.0",
"@sentry/node": "^9.0.1",
"axios": "^1.7.9",
"colors": "1.4.0",
"compression": "1.7.5",
"compression": "1.8.0",
"cors": "2.8.5",
"crypto-js": "4.2.0",
"discord.js": "^14.17.3",
"discord.js": "^14.18.0",
"dompurify": "^3.2.4",
"express": "4.21.2",
"express-openid-connect": "^2.17.1",
Expand Down
2 changes: 1 addition & 1 deletion public/js/Brand.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ let BRAND = {
},
about: {
imageUrl: '../images/mirotalk-logo.gif',
title: '<strong>WebRTC SFU v1.7.26</strong>',
title: '<strong>WebRTC SFU v1.7.27</strong>',
html: `
<button
id="support-button"
Expand Down
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.7.26
* @version 1.7.27
*
*/

Expand Down Expand Up @@ -4905,7 +4905,7 @@ function showAbout() {
position: 'center',
imageUrl: BRAND.about?.imageUrl && BRAND.about.imageUrl.trim() !== '' ? BRAND.about.imageUrl : image.about,
customClass: { image: 'img-about' },
title: BRAND.about?.title && BRAND.about.title.trim() !== '' ? BRAND.about.title : 'WebRTC SFU v1.7.26',
title: BRAND.about?.title && BRAND.about.title.trim() !== '' ? BRAND.about.title : 'WebRTC SFU v1.7.27',
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.7.26
* @version 1.7.27
*
*/

Expand Down
111 changes: 111 additions & 0 deletions tests/test-OpenRedirect.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
const sinon = require('sinon');

describe('test-OpenRedirect', function () {
let req, res, next, log;

beforeEach(() => {
req = { path: '', url: '', headers: {}, body: {} };
res = {
status: sinon.stub().returnsThis(),
send: sinon.stub(),
redirect: sinon.stub(),
};
next = sinon.spy();
log = { error: sinon.spy() }; // Mock the logger
});

// Middleware function to test
const middleware = (err, req, res, next) => {
if (err && (err instanceof SyntaxError || err.status === 400 || 'body' in err)) {
log.error('Request Error', {
header: req.headers,
body: req.body,
error: err.message,
});
return res.status(400).send({ status: 404, message: err.message }); // Bad request
}

// Prevent open redirect attacks by checking if the path is an external domain
const cleanPath = req.path.replace(/^\/+/, '');
if (/^([a-zA-Z0-9-]+\.)+[a-zA-Z]{2,}/.test(cleanPath)) {
return res.status(400).send('Bad Request: Potential Open Redirect Detected');
}

if (req.path.endsWith('/') && req.path.length > 1) {
let query = req.url.substring(req.path.length).replace(/\/$/, ''); // Ensure query params don't end in '/'
return res.redirect(301, req.path.slice(0, -1) + query);
}

next();
};

it('should prevent open redirect attempts', function () {
req.path = '//google.com/';

middleware(null, req, res, next);

res.status.calledOnceWithExactly(400).should.be.true();
res.send.calledOnceWithExactly('Bad Request: Potential Open Redirect Detected').should.be.true();
});

it('should handle query parameters correctly when removing trailing slash', function () {
req.path = '/join/';
req.url = '/join/?room=4b874c64-a8bd-4a82-a91e-53acc420b4b3uch/';

middleware(null, req, res, next);

res.redirect.calledOnce.should.be.true();
res.redirect.calledWith(301, '/join?room=4b874c64-a8bd-4a82-a91e-53acc420b4b3uch').should.be.true();
});

it('should handle query parameters correctly', function () {
req.path = '/join/';
req.url = '/join/?room=123';

middleware(null, req, res, next);

res.redirect.calledOnce.should.be.true();
res.redirect.calledWith(301, '/join?room=123').should.be.true();
});

it('should handle query parameters correctly', function () {
req.path = '/join/';
req.url =
'/join/?room=test&roomPassword=0&name=mirotalksfu&audio=1&video=1&screen=0&hide=0&notify=1&duration=00:00:30';

middleware(null, req, res, next);

res.redirect.calledOnce.should.be.true();
res.redirect
.calledWith(
301,
'/join?room=test&roomPassword=0&name=mirotalksfu&audio=1&video=1&screen=0&hide=0&notify=1&duration=00:00:30',
)
.should.be.true();
});

it('should handle query parameters with token', function () {
req.path = '/join/';
req.url =
'/join/?room=test&roomPassword=0&name=mirotalksfu&audio=1&video=1&screen=0&hide=0&notify=0&token=token';

middleware(null, req, res, next);

res.redirect.calledOnce.should.be.true();
res.redirect
.calledWith(
301,
'/join?room=test&roomPassword=0&name=mirotalksfu&audio=1&video=1&screen=0&hide=0&notify=0&token=token',
)
.should.be.true();
});

it('should call next() if no conditions are met', function () {
req.path = '/valid-path';
req.url = '/valid-path';

middleware(null, req, res, next);

next.calledOnce.should.be.true();
});
});

0 comments on commit 775307d

Please sign in to comment.