Skip to content

Commit 58c2a5e

Browse files
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
1 parent d203fef commit 58c2a5e

4 files changed

Lines changed: 142 additions & 4 deletions

File tree

js/ip_access_control.js

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,27 @@ function isAllowed (clientIp, whitelist) {
3333
}
3434
}
3535

36+
/**
37+
* Resolves a client IP for both Express and Socket.IO requests.
38+
* If the direct peer is loopback, trust the first X-Forwarded-For value (local reverse proxy case).
39+
* Otherwise ignore X-Forwarded-For to prevent spoofing.
40+
* @param {object} req - Incoming request object (Express request or Socket.IO handshake request)
41+
* @returns {string} The resolved client IP address
42+
*/
43+
function resolveClientIp (req) {
44+
const directIp = req.socket?.remoteAddress || req.connection?.remoteAddress || req.ip;
45+
const LOOPBACK_WHITELIST = ["127.0.0.1", "::ffff:127.0.0.1", "::1"];
46+
47+
if (isAllowed(directIp, LOOPBACK_WHITELIST)) {
48+
const forwardedFor = req.headers?.["x-forwarded-for"];
49+
if (typeof forwardedFor === "string" && forwardedFor.trim().length > 0) {
50+
return forwardedFor.split(",")[0].trim();
51+
}
52+
}
53+
54+
return directIp;
55+
}
56+
3657
/**
3758
* Creates an Express middleware for IP whitelisting
3859
* @param {string[]} whitelist - Array of allowed IP addresses or CIDR ranges
@@ -48,16 +69,41 @@ function ipAccessControl (whitelist) {
4869
}
4970

5071
return function (req, res, next) {
51-
const clientIp = req.ip || req.socket.remoteAddress;
72+
const clientIp = resolveClientIp(req);
5273

5374
if (isAllowed(clientIp, whitelist)) {
5475
res.header("Access-Control-Allow-Origin", "*");
5576
next();
5677
} else {
57-
Log.log(`IP ${clientIp} is not allowed to access the mirror`);
78+
Log.warn(`IP ${clientIp} is not allowed to access the mirror`);
5879
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.");
5980
}
6081
};
6182
}
6283

63-
module.exports = { ipAccessControl };
84+
/**
85+
* Creates a Socket.IO `allowRequest` handler that enforces the same IP whitelist as the HTTP middleware.
86+
* This closes the gap where Socket.IO handshakes bypassed the Express-only `ipAccessControl` middleware.
87+
* @param {string[]} whitelist - Array of allowed IP addresses or CIDR ranges
88+
* @returns {(req: object, callback: (err: string | null, success: boolean) => void) => void} Socket.IO allowRequest handler
89+
*/
90+
function socketIpAccessControl (whitelist) {
91+
// Empty whitelist means allow all
92+
if (!Array.isArray(whitelist) || whitelist.length === 0) {
93+
return function (req, callback) {
94+
callback(null, true); // allow the connection
95+
};
96+
}
97+
98+
return function (req, callback) {
99+
const clientIp = resolveClientIp(req);
100+
if (isAllowed(clientIp, whitelist)) {
101+
callback(null, true); // allow the connection
102+
} else {
103+
Log.warn(`IP ${clientIp} is not allowed to connect to the mirror socket`);
104+
callback("This device is not allowed to access your mirror.", false);
105+
}
106+
};
107+
}
108+
109+
module.exports = { ipAccessControl, socketIpAccessControl };

js/server.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ const helmet = require("helmet");
77
const socketio = require("socket.io");
88
const Log = require("logger");
99

10-
const { ipAccessControl } = require("./ip_access_control");
10+
const { ipAccessControl, socketIpAccessControl } = require("./ip_access_control");
1111

1212
const vendor = require("./vendor");
1313

@@ -41,6 +41,7 @@ function Server (configObj) {
4141
server = http.Server(app);
4242
}
4343
const io = socketio(server, {
44+
allowRequest: socketIpAccessControl(config.ipWhitelist),
4445
cors: {
4546
origin: /.*$/,
4647
credentials: true

tests/e2e/ipWhitelist_spec.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ describe("ipWhitelist directive configuration", () => {
1515
const res = await fetch(`http://localhost:${port}`);
1616
expect(res.status).toBe(403);
1717
});
18+
19+
it("should also reject Socket.IO handshake with 403 (Forbidden) — not just HTTP routes", async () => {
20+
const port = global.testPort || 8080;
21+
const res = await fetch(`http://localhost:${port}/socket.io/?EIO=4&transport=polling`);
22+
expect(res.status).toBe(403);
23+
});
1824
});
1925

2026
describe("When whitelist is empty (allow all IPs)", () => {
@@ -31,5 +37,11 @@ describe("ipWhitelist directive configuration", () => {
3137
const res = await fetch(`http://localhost:${port}`);
3238
expect(res.status).toBe(200);
3339
});
40+
41+
it("should also allow Socket.IO handshake with 200 (OK) — not just HTTP routes", async () => {
42+
const port = global.testPort || 8080;
43+
const res = await fetch(`http://localhost:${port}/socket.io/?EIO=4&transport=polling`);
44+
expect(res.status).toBe(200);
45+
});
3446
});
3547
});
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
3+
import { ipAccessControl, socketIpAccessControl } from "../../../js/ip_access_control";
4+
5+
/**
6+
* Creates a minimal Express-like response mock used by the middleware tests.
7+
* @returns {{ header: ReturnType<typeof vi.fn>, status: ReturnType<typeof vi.fn>, send: ReturnType<typeof vi.fn> }} Mock response object.
8+
*/
9+
function createResponseMock () {
10+
return {
11+
header: vi.fn(),
12+
status: vi.fn(function () {
13+
return this;
14+
}),
15+
send: vi.fn()
16+
};
17+
}
18+
19+
describe("ip_access_control", () => {
20+
describe("ipAccessControl", () => {
21+
it("trusts first X-Forwarded-For entry when direct peer is loopback", () => {
22+
const middleware = ipAccessControl(["203.0.113.10"]);
23+
const req = {
24+
socket: { remoteAddress: "127.0.0.1" },
25+
headers: { "x-forwarded-for": "203.0.113.10, 10.0.0.2" }
26+
};
27+
const res = createResponseMock();
28+
const next = vi.fn();
29+
30+
middleware(req, res, next);
31+
32+
expect(next).toHaveBeenCalledOnce();
33+
expect(res.status).not.toHaveBeenCalled();
34+
});
35+
36+
it("ignores X-Forwarded-For when direct peer is not loopback", () => {
37+
const middleware = ipAccessControl(["203.0.113.10"]);
38+
const req = {
39+
socket: { remoteAddress: "198.51.100.7" },
40+
headers: { "x-forwarded-for": "203.0.113.10" }
41+
};
42+
const res = createResponseMock();
43+
const next = vi.fn();
44+
45+
middleware(req, res, next);
46+
47+
expect(next).not.toHaveBeenCalled();
48+
expect(res.status).toHaveBeenCalledWith(403);
49+
});
50+
});
51+
52+
describe("socketIpAccessControl", () => {
53+
it("accepts socket handshake using forwarded client IP when direct peer is loopback", () => {
54+
const allowRequest = socketIpAccessControl(["203.0.113.10"]);
55+
const req = {
56+
socket: { remoteAddress: "::1" },
57+
headers: { "x-forwarded-for": "203.0.113.10, 10.0.0.2" }
58+
};
59+
const callback = vi.fn();
60+
61+
allowRequest(req, callback);
62+
63+
expect(callback).toHaveBeenCalledWith(null, true);
64+
});
65+
66+
it("rejects socket handshake when only forwarded IP matches whitelist", () => {
67+
const allowRequest = socketIpAccessControl(["203.0.113.10"]);
68+
const req = {
69+
socket: { remoteAddress: "198.51.100.7" },
70+
headers: { "x-forwarded-for": "203.0.113.10" }
71+
};
72+
const callback = vi.fn();
73+
74+
allowRequest(req, callback);
75+
76+
expect(callback).toHaveBeenCalledWith("This device is not allowed to access your mirror.", false);
77+
});
78+
});
79+
});

0 commit comments

Comments
 (0)