CVE-2023-2850: Origin Validation Error (CWE-346)
value: NodeBB is affected by a Cross-Site WebSocket Hijacking vulnerability due to missing validation of the request origin. Exploitation of this vulnerability allows certain user information to be extracted by attacker.
CWE description: The product does not properly verify that the source of data or communication is valid.
CVSS v3 base score: 4.7
Published: 2023-07-25T12:15Z
Repository: https://github.com/NodeBB/NodeBB
Language: JavaScript
Files changed: public/src/sockets.js, src/middleware/csrf.js, src/socket.io/index.js, test/helpers/index.js, test/socket.io.js
Commit date: 2023-05-15 11:15:53 -0400
Commit message: poc: use csrf_token in ws handshake (#11573)


Diff: diff --git a/public/src/sockets.js b/public/src/sockets.js
index 6d3ccfbb808..acd3ba3dbe9 100644
--- a/public/src/sockets.js
+++ b/public/src/sockets.js
@@ -13,10 +13,13 @@ app = window.app || {};
 	const ioParams = {
 		reconnectionAttempts: config.maxReconnectionAttempts,
 		reconnectionDelay: config.reconnectionDelay,
 		transports: config.socketioTransports,
 		path: config.relative_path + '/socket.io',
+		query: {
+			_csrf: config.csrf_token,
+		},
 	};
 
 	window.socket = io(config.websocketAddress, ioParams);
 
 	const oEmit = socket.emit;
diff --git a/src/middleware/csrf.js b/src/middleware/csrf.js
index f6af0c625bf..be5b0761fe5 100644
--- a/src/middleware/csrf.js
+++ b/src/middleware/csrf.js
@@ -3,20 +3,24 @@
 const { csrfSync } = require('csrf-sync');
 
 const {
 	generateToken,
 	csrfSynchronisedProtection,
+	isRequestValid,
 } = csrfSync({
 	getTokenFromRequest: (req) => {
 		if (req.headers['x-csrf-token']) {
 			return req.headers['x-csrf-token'];
-		} else if (req.body.csrf_token) {
+		} else if (req.body && req.body.csrf_token) {
 			return req.body.csrf_token;
+		} else if (req.query) {
+			return req.query._csrf;
 		}
 	},
 	size: 64,
 });
 
 module.exports = {
 	generateToken,
 	csrfSynchronisedProtection,
+	isRequestValid,
 };
diff --git a/src/socket.io/index.js b/src/socket.io/index.js
index 963267ed9a8..d457afefbc4 100644
--- a/src/socket.io/index.js
+++ b/src/socket.io/index.js
@@ -32,17 +32,29 @@ Sockets.init = async function (server) {
 		} else {
 			winston.warn('clustering detected, you should setup redis!');
 		}
 	}
 
-	io.use(authorize);
-
 	io.on('connection', onConnection);
 
 	const opts = {
 		transports: nconf.get('socket.io:transports') || ['polling', 'websocket'],
 		cookie: false,
+		allowRequest: (req, callback) => {
+			authorize(req, (err) => {
+				if (err) {
+					return callback(err);
+				}
+				const csrf = require('../middleware/csrf');
+				const isValid = csrf.isRequestValid({
+					session: req.session || {},
+					query: req._query,
+					headers: req.headers,
+				});
+				callback(null, isValid);
+			});
+		},
 	};
 	/*
 	 * Restrict socket.io listener to cookie domain. If none is set, infer based on url.
 	 * Production only so you don't get accidentally locked out.
 	 * Can be overridden via config (socket.io:origins)
@@ -60,11 +72,15 @@ Sockets.init = async function (server) {
 	io.listen(server, opts);
 	Sockets.server = io;
 };
 
 function onConnection(socket) {
-	socket.ip = (socket.request.headers['x-forwarded-for'] || socket.request.connection.remoteAddress || '').split(',')[0];
+	socket.uid = socket.request.uid;
+	socket.ip = (
+		socket.request.headers['x-forwarded-for'] ||
+		socket.request.connection.remoteAddress || ''
+	).split(',')[0];
 	socket.request.ip = socket.ip;
 	logger.io_one(socket, socket.uid);
 
 	onConnect(socket);
 	socket.onAny((event, ...args) => {
@@ -229,13 +245,11 @@ async function validateSession(socket, errorMsg) {
 	});
 }
 
 const cookieParserAsync = util.promisify((req, callback) => cookieParser(req, {}, err => callback(err)));
 
-async function authorize(socket, callback) {
-	const { request } = socket;
-
+async function authorize(request, callback) {
 	if (!request) {
 		return callback(new Error('[[error:not-authorized]]'));
 	}
 
 	await cookieParserAsync(request);
@@ -244,19 +258,17 @@ async function authorize(socket, callback) {
 		sessionId: request.signedCookies ? request.signedCookies[nconf.get('sessionKey')] : null,
 		request: request,
 	});
 
 	const sessionData = await getSessionAsync(sessionId);
-
+	request.session = sessionData;
+	let uid = 0;
 	if (sessionData && sessionData.passport && sessionData.passport.user) {
-		request.session = sessionData;
-		socket.uid = parseInt(sessionData.passport.user, 10);
-	} else {
-		socket.uid = 0;
+		uid = parseInt(sessionData.passport.user, 10);
 	}
-	request.uid = socket.uid;
-	callback();
+	request.uid = uid;
+	callback(null, uid);
 }
 
 Sockets.in = function (room) {
 	return Sockets.server && Sockets.server.in(room);
 };
diff --git a/test/helpers/index.js b/test/helpers/index.js
index b79bf66e2d6..4adcb1b0dd9 100644
--- a/test/helpers/index.js
+++ b/test/helpers/index.js
@@ -94,21 +94,24 @@ helpers.logoutUser = function (jar, callback) {
 			callback(err, response, body);
 		});
 	});
 };
 
-helpers.connectSocketIO = function (res, callback) {
+helpers.connectSocketIO = function (res, csrf_token, callback) {
 	const io = require('socket.io-client');
 	let cookies = res.headers['set-cookie'];
 	cookies = cookies.filter(c => /express.sid=[^;]+;/.test(c));
 	const cookie = cookies[0];
 	const socket = io(nconf.get('base_url'), {
 		path: `${nconf.get('relative_path')}/socket.io`,
 		extraHeaders: {
 			Origin: nconf.get('url'),
 			Cookie: cookie,
 		},
+		query: {
+			_csrf: csrf_token,
+		},
 	});
 
 	socket.on('connect', () => {
 		callback(null, socket);
 	});
diff --git a/test/socket.io.js b/test/socket.io.js
index 726f6f71adb..4e2292d4e56 100644
--- a/test/socket.io.js
+++ b/test/socket.io.js
@@ -71,11 +71,11 @@ describe('socket.io', () => {
 				},
 				json: true,
 			}, (err, res) => {
 				assert.ifError(err);
 
-				helpers.connectSocketIO(res, (err, _io) => {
+				helpers.connectSocketIO(res, body.csrf_token, (err, _io) => {
 					io = _io;
 					assert.ifError(err);
 
 					done();
 				});

Vulnerable code: 		} else if (req.body.csrf_token) {
	io.use(authorize);

	socket.ip = (socket.request.headers['x-forwarded-for'] || socket.request.connection.remoteAddress || '').split(',')[0];
async function authorize(socket, callback) {
	const { request } = socket;


		request.session = sessionData;
		socket.uid = parseInt(sessionData.passport.user, 10);
	} else {
		socket.uid = 0;
	request.uid = socket.uid;
	callback();
helpers.connectSocketIO = function (res, callback) {
				helpers.connectSocketIO(res, (err, _io) => {
Fixed code: 		query: {
			_csrf: config.csrf_token,
		},
	isRequestValid,
		} else if (req.body && req.body.csrf_token) {
		} else if (req.query) {
			return req.query._csrf;
	isRequestValid,
		allowRequest: (req, callback) => {
			authorize(req, (err) => {
				if (err) {
					return callback(err);
				}
				const csrf = require('../middleware/csrf');
				const isValid = csrf.isRequestValid({
					session: req.session || {},
					query: req._query,
					headers: req.headers,
				});
				callback(null, isValid);
			});
		},
	socket.uid = socket.request.uid;
	socket.ip = (
		socket.request.headers['x-forwarded-for'] ||
		socket.request.connection.remoteAddress || ''
	).split(',')[0];
async function authorize(request, callback) {
	request.session = sessionData;
	let uid = 0;
		uid = parseInt(sessionData.passport.user, 10);
	request.uid = uid;
	callback(null, uid);
helpers.connectSocketIO = function (res, csrf_token, callback) {
		query: {
			_csrf: csrf_token,
		},
				helpers.connectSocketIO(res, body.csrf_token, (err, _io) => {