fix(server): enforce ipWhitelist for Socket.IO too (#4169)

ipWhitelist was only applied to HTTP routes, so Socket.IO module
namespaces could still be reached from disallowed clients.

This adds the same whitelist check to Socket.IO handshakes
(allowRequest), and reuses the same client IP resolution for both HTTP
and Socket.IO (forwarded IP is only trusted for loopback peers).

Also adds tests for handshake allow/deny and forwarded-header behavior.

Fixes: GHSA-w26r-fwg8-rcp3
This commit is contained in:
Kristjan ESPERANTO
2026-06-01 17:26:16 +02:00
committed by GitHub
parent d203fefed1
commit 58c2a5e675
4 changed files with 142 additions and 4 deletions

View File

@@ -33,6 +33,27 @@ function isAllowed (clientIp, whitelist) {
}
}
/**
* Resolves a client IP for both Express and Socket.IO requests.
* If the direct peer is loopback, trust the first X-Forwarded-For value (local reverse proxy case).
* Otherwise ignore X-Forwarded-For to prevent spoofing.
* @param {object} req - Incoming request object (Express request or Socket.IO handshake request)
* @returns {string} The resolved client IP address
*/
function resolveClientIp (req) {
const directIp = req.socket?.remoteAddress || req.connection?.remoteAddress || req.ip;
const LOOPBACK_WHITELIST = ["127.0.0.1", "::ffff:127.0.0.1", "::1"];
if (isAllowed(directIp, LOOPBACK_WHITELIST)) {
const forwardedFor = req.headers?.["x-forwarded-for"];
if (typeof forwardedFor === "string" && forwardedFor.trim().length > 0) {
return forwardedFor.split(",")[0].trim();
}
}
return directIp;
}
/**
* Creates an Express middleware for IP whitelisting
* @param {string[]} whitelist - Array of allowed IP addresses or CIDR ranges
@@ -48,16 +69,41 @@ function ipAccessControl (whitelist) {
}
return function (req, res, next) {
const clientIp = req.ip || req.socket.remoteAddress;
const clientIp = resolveClientIp(req);
if (isAllowed(clientIp, whitelist)) {
res.header("Access-Control-Allow-Origin", "*");
next();
} else {
Log.log(`IP ${clientIp} is not allowed to access the mirror`);
Log.warn(`IP ${clientIp} is not allowed to access the mirror`);
res.status(403).send("This device is not allowed to access your mirror. <br> Please check your config.js or config.js.sample to change this.");
}
};
}
module.exports = { ipAccessControl };
/**
* Creates a Socket.IO `allowRequest` handler that enforces the same IP whitelist as the HTTP middleware.
* This closes the gap where Socket.IO handshakes bypassed the Express-only `ipAccessControl` middleware.
* @param {string[]} whitelist - Array of allowed IP addresses or CIDR ranges
* @returns {(req: object, callback: (err: string | null, success: boolean) => void) => void} Socket.IO allowRequest handler
*/
function socketIpAccessControl (whitelist) {
// Empty whitelist means allow all
if (!Array.isArray(whitelist) || whitelist.length === 0) {
return function (req, callback) {
callback(null, true); // allow the connection
};
}
return function (req, callback) {
const clientIp = resolveClientIp(req);
if (isAllowed(clientIp, whitelist)) {
callback(null, true); // allow the connection
} else {
Log.warn(`IP ${clientIp} is not allowed to connect to the mirror socket`);
callback("This device is not allowed to access your mirror.", false);
}
};
}
module.exports = { ipAccessControl, socketIpAccessControl };

View File

@@ -7,7 +7,7 @@ const helmet = require("helmet");
const socketio = require("socket.io");
const Log = require("logger");
const { ipAccessControl } = require("./ip_access_control");
const { ipAccessControl, socketIpAccessControl } = require("./ip_access_control");
const vendor = require("./vendor");
@@ -41,6 +41,7 @@ function Server (configObj) {
server = http.Server(app);
}
const io = socketio(server, {
allowRequest: socketIpAccessControl(config.ipWhitelist),
cors: {
origin: /.*$/,
credentials: true

View File

@@ -15,6 +15,12 @@ describe("ipWhitelist directive configuration", () => {
const res = await fetch(`http://localhost:${port}`);
expect(res.status).toBe(403);
});
it("should also reject Socket.IO handshake with 403 (Forbidden) — not just HTTP routes", async () => {
const port = global.testPort || 8080;
const res = await fetch(`http://localhost:${port}/socket.io/?EIO=4&transport=polling`);
expect(res.status).toBe(403);
});
});
describe("When whitelist is empty (allow all IPs)", () => {
@@ -31,5 +37,11 @@ describe("ipWhitelist directive configuration", () => {
const res = await fetch(`http://localhost:${port}`);
expect(res.status).toBe(200);
});
it("should also allow Socket.IO handshake with 200 (OK) — not just HTTP routes", async () => {
const port = global.testPort || 8080;
const res = await fetch(`http://localhost:${port}/socket.io/?EIO=4&transport=polling`);
expect(res.status).toBe(200);
});
});
});

View File

@@ -0,0 +1,79 @@
import { describe, expect, it, vi } from "vitest";
import { ipAccessControl, socketIpAccessControl } from "../../../js/ip_access_control";
/**
* Creates a minimal Express-like response mock used by the middleware tests.
* @returns {{ header: ReturnType<typeof vi.fn>, status: ReturnType<typeof vi.fn>, send: ReturnType<typeof vi.fn> }} Mock response object.
*/
function createResponseMock () {
return {
header: vi.fn(),
status: vi.fn(function () {
return this;
}),
send: vi.fn()
};
}
describe("ip_access_control", () => {
describe("ipAccessControl", () => {
it("trusts first X-Forwarded-For entry when direct peer is loopback", () => {
const middleware = ipAccessControl(["203.0.113.10"]);
const req = {
socket: { remoteAddress: "127.0.0.1" },
headers: { "x-forwarded-for": "203.0.113.10, 10.0.0.2" }
};
const res = createResponseMock();
const next = vi.fn();
middleware(req, res, next);
expect(next).toHaveBeenCalledOnce();
expect(res.status).not.toHaveBeenCalled();
});
it("ignores X-Forwarded-For when direct peer is not loopback", () => {
const middleware = ipAccessControl(["203.0.113.10"]);
const req = {
socket: { remoteAddress: "198.51.100.7" },
headers: { "x-forwarded-for": "203.0.113.10" }
};
const res = createResponseMock();
const next = vi.fn();
middleware(req, res, next);
expect(next).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(403);
});
});
describe("socketIpAccessControl", () => {
it("accepts socket handshake using forwarded client IP when direct peer is loopback", () => {
const allowRequest = socketIpAccessControl(["203.0.113.10"]);
const req = {
socket: { remoteAddress: "::1" },
headers: { "x-forwarded-for": "203.0.113.10, 10.0.0.2" }
};
const callback = vi.fn();
allowRequest(req, callback);
expect(callback).toHaveBeenCalledWith(null, true);
});
it("rejects socket handshake when only forwarded IP matches whitelist", () => {
const allowRequest = socketIpAccessControl(["203.0.113.10"]);
const req = {
socket: { remoteAddress: "198.51.100.7" },
headers: { "x-forwarded-for": "203.0.113.10" }
};
const callback = vi.fn();
allowRequest(req, callback);
expect(callback).toHaveBeenCalledWith("This device is not allowed to access your mirror.", false);
});
});
});