mirror of
https://github.com/MichMich/MagicMirror.git
synced 2026-06-13 19:25:43 +00:00
This PR attempts to fix the unauthorized secret expansion vulnerability reported in [GHSA-q4gh-4ffp-5cg8](https://github.com/MagicMirrorOrg/MagicMirror/security/advisories/GHSA-q4gh-4ffp-5cg8). Previously, if a module sent a payload through the socket containing any `**SECRET_FOO**` placeholder, the server would unconditionally expand it with the real environment variable. This meant a manipulated module could theoretically extract secrets that belonged to other modules. To prevent this, the expansion logic is now much stricter and scoped to the individual module: * In `app.js`, we now store a copy of the redacted config (`global.configRedacted`) to keep track of which module uses which secrets. * In `node_helper.js`, before handling a socket notification, we build a specific "allow-list" (`Set`) of secrets that are actually present in the calling module's config. * `replaceSecretPlaceholder` in `server_functions.js` was updated to accept this `Set` and will now only expand placeholders that the module is explicitly authorized to know. Unlisted placeholders are safely ignored. I also updated the unit tests to cover the new allow-list behavior. Since this security stuff is tricky and gives me headaches all the time, I've added more comments than usual. I've tried several ways to make it a little simpler, but unfortunately, I couldn't come up with anything easier than that. I'd appreciate it if someone could take a critical look at the logic to make sure I didn't miss anything!
179 lines
5.1 KiB
JavaScript
179 lines
5.1 KiB
JavaScript
const express = require("express");
|
|
const Log = require("logger");
|
|
const { replaceSecretPlaceholder } = require("#server_functions");
|
|
|
|
/**
|
|
* Determine which secrets a module is allowed to restore. A module may only
|
|
* restore the `**SECRET_***` placeholders that appear in its own config — the
|
|
* exact inverse of how the config is redacted before it is sent to the browser.
|
|
* @param {string} moduleName - Name of the module.
|
|
* @returns {Set<string>} The secret names the module may restore.
|
|
*/
|
|
function getAllowedSecrets (moduleName) {
|
|
const modules = global.configRedacted?.modules || [];
|
|
const moduleConfig = modules.find((m) => m.module === moduleName);
|
|
const allowed = new Set();
|
|
if (moduleConfig) {
|
|
// Stringify the config to easily find all expected **SECRET_*** placeholders
|
|
for (const [, secretName] of JSON.stringify(moduleConfig).matchAll(/\*\*(SECRET_[^*]+)\*\*/g)) {
|
|
allowed.add(secretName);
|
|
}
|
|
}
|
|
return allowed;
|
|
}
|
|
|
|
class NodeHelper {
|
|
init () {
|
|
Log.log("Initializing new module helper ...");
|
|
}
|
|
|
|
loaded () {
|
|
Log.log(`Module helper loaded: ${this.name}`);
|
|
}
|
|
|
|
start () {
|
|
Log.log(`Starting module helper: ${this.name}`);
|
|
}
|
|
|
|
/**
|
|
* Called when the MagicMirror² server receives a `SIGINT`
|
|
* Close any open connections, stop any sub-processes and
|
|
* gracefully exit the module.
|
|
*/
|
|
stop () {
|
|
Log.log(`Stopping module helper: ${this.name}`);
|
|
}
|
|
|
|
/**
|
|
* This method is called when a socket notification arrives.
|
|
* @param {string} notification The identifier of the notification.
|
|
* @param {object} payload The payload of the notification.
|
|
*/
|
|
socketNotificationReceived (notification, payload) {
|
|
Log.log(`${this.name} received a socket notification: ${notification} - Payload: ${payload}`);
|
|
}
|
|
|
|
/**
|
|
* Set the module name.
|
|
* @param {string} name Module name.
|
|
*/
|
|
setName (name) {
|
|
this.name = name;
|
|
}
|
|
|
|
/**
|
|
* Set the module path.
|
|
* @param {string} path Module path.
|
|
*/
|
|
setPath (path) {
|
|
this.path = path;
|
|
}
|
|
|
|
/*
|
|
* sendSocketNotification(notification, payload)
|
|
* Send a socket notification to the node helper.
|
|
*
|
|
* argument notification string - The identifier of the notification.
|
|
* argument payload mixed - The payload of the notification.
|
|
*/
|
|
sendSocketNotification (notification, payload) {
|
|
this.io.of(this.name).emit(notification, payload);
|
|
}
|
|
|
|
/*
|
|
* setExpressApp(app)
|
|
* Sets the express app object for this module.
|
|
* This allows you to host files from the created webserver.
|
|
*
|
|
* argument app Express app - The Express app object.
|
|
*/
|
|
setExpressApp (app) {
|
|
this.expressApp = app;
|
|
|
|
app.use(`/${this.name}`, express.static(`${this.path}/public`));
|
|
}
|
|
|
|
/*
|
|
* setSocketIO(io)
|
|
* Sets the socket io object for this module.
|
|
* Binds message receiver.
|
|
*
|
|
* argument io Socket.io - The Socket io object.
|
|
*/
|
|
setSocketIO (io) {
|
|
this.io = io;
|
|
|
|
Log.log(`Connecting socket for: ${this.name}`);
|
|
|
|
io.of(this.name).on("connection", (socket) => {
|
|
// register catch all.
|
|
socket.onAny((notification, payload) => {
|
|
if (config?.hideConfigSecrets && payload && typeof payload === "object") {
|
|
try {
|
|
// Calculate exactly which secrets this module is allowed to receive
|
|
const allowedSecrets = getAllowedSecrets(this.name);
|
|
// Expand only these safe, module-specific secrets in the payload
|
|
const payloadStr = replaceSecretPlaceholder(JSON.stringify(payload), allowedSecrets);
|
|
this.socketNotificationReceived(notification, JSON.parse(payloadStr));
|
|
} catch (e) {
|
|
Log.error("Error substituting variables in payload: ", e);
|
|
this.socketNotificationReceived(notification, payload);
|
|
}
|
|
} else {
|
|
this.socketNotificationReceived(notification, payload);
|
|
}
|
|
});
|
|
});
|
|
}
|
|
|
|
/**
|
|
* Check the status of a fetch response.
|
|
* @param {Response} response The fetch response.
|
|
* @returns {Response} The fetch response if ok.
|
|
*/
|
|
static checkFetchStatus (response) {
|
|
// response.status >= 200 && response.status < 300
|
|
if (response.ok) {
|
|
return response;
|
|
} else {
|
|
throw Error(response.statusText);
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Look at the specified error and return an appropriate error type, that
|
|
* can be translated to a detailed error message
|
|
* @param {Error} error the error from fetching something
|
|
* @returns {string} the string of the detailed error message in the translations
|
|
*/
|
|
static checkFetchError (error) {
|
|
let error_type = "MODULE_ERROR_UNSPECIFIED";
|
|
if (error.code === "EAI_AGAIN") {
|
|
error_type = "MODULE_ERROR_NO_CONNECTION";
|
|
} else {
|
|
const message = typeof error.message === "string" ? error.message.toLowerCase() : "";
|
|
if (message.includes("unauthorized") || message.includes("http 401") || message.includes("http 403")) {
|
|
error_type = "MODULE_ERROR_UNAUTHORIZED";
|
|
}
|
|
}
|
|
return error_type;
|
|
}
|
|
|
|
/**
|
|
* Create a new NodeHelper subclass with the given module definition.
|
|
* @param {object} moduleDefinition Methods and properties for the helper.
|
|
* @returns {typeof NodeHelper} A new subclass of NodeHelper.
|
|
*/
|
|
static create (moduleDefinition) {
|
|
return class extends NodeHelper {
|
|
constructor () {
|
|
super();
|
|
Object.assign(this, moduleDefinition);
|
|
this.init();
|
|
}
|
|
};
|
|
}
|
|
}
|
|
|
|
module.exports = NodeHelper;
|