Skip to content
Permalink
Browse files Browse the repository at this point in the history
prevent XSS via admin-notifies system
The admin-notifies system takes user input from the X-Forwarded-For by header,
and injects it into html rendered by the browser in the admin-notification
system.  This patch appropriately escapes all such content injected into the
notification drawers.  This patch should prevent XSS attacks designed to
exploit user credentials or other access.  Thanks to Deja Vu Security -
Accenture, and Damien Sudol for reporting.
  • Loading branch information
bewest committed Jun 22, 2021
1 parent 8d75200 commit 68f3f90
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
19 changes: 14 additions & 5 deletions lib/client/adminnotifiesclient.js
Expand Up @@ -36,8 +36,10 @@ function init (client, $) {

notifies.updateAdminNotifies();

function wrapmessage(title, message, count, ago, persistent) {
let html = '<hr><p><b>' + title + '</b></p><p class="adminNotifyMessage">' + message + '</p>';
function wrapmessage(title, message, count, ago, persistent, seeAlso) {
let html = $('<div><div><hr><p><b class="adminNotifyTitle"></b></p><p class="adminNotifyMessage"></p></div><p class="adminNotifyMessageAdditionalInfo"></p></div>');
html.find('.adminNotifyTitle').text(title);
html.find('.adminNotifyMessage').text(message);

let additional = '';

Expand All @@ -51,8 +53,11 @@ function init (client, $) {
if (ago == 0) { ago = client.translate('less than 1'); }
if (!persistent && ago) additional += client.translate('Last recorded %1 %2 ago.', ago, units);

if (additional) html += '<p class="adminNotifyMessageAdditionalInfo">' + additional + '</p>'
return html;
html.find('.adminNotifyMessageAdditionalInfo').text(additional);
if (seeAlso && init.addendums[seeAlso]) {
html.append($(init.addendums[seeAlso]));
}
return html.html( );
}

notifies.prepare = function prepare() {
Expand All @@ -69,7 +74,7 @@ function init (client, $) {
/* eslint-disable-next-line security/detect-object-injection */ // verified false positive
var m = messages[i];
const ago = Math.round((Date.now() - m.lastRecorded) / 60000);
html += wrapmessage(translate(m.title), translate(m.message), m.count, ago, m.persistent);
html += wrapmessage(translate(m.title), translate(m.message), m.count, ago, m.persistent, m.seeAlso);
}
} else {
if (messageCount > 0) {
Expand Down Expand Up @@ -99,5 +104,9 @@ function init (client, $) {
return notifies;

}
init.addendums = {
disableWorldReadable: '<p>Please consider closing access to the site by following the instructions in the <a href="http://nightscout.github.io/nightscout/security/#how-to-turn-off-unauthorized-access" target="_new">Nightscout documentation</a>.</p>'

};

module.exports = init;
3 changes: 2 additions & 1 deletion lib/server/bootevent.js
Expand Up @@ -121,8 +121,9 @@ function boot (env, language) {
if (env.settings.authDefaultRoles == 'readable') {
const message = {
title: "Nightscout readable by world"
,message: "Your Nightscout installation is readable by anyone who knows the web page URL. Please consider closing access to the site by following the instructions in the <a href=\"http://nightscout.github.io/nightscout/security/#how-to-turn-off-unauthorized-access\" target=\"_new\">Nightscout documentation</a>."

This comment has been minimized.

Copy link
@sulkaharo

sulkaharo Jun 22, 2021

Member

Why was this changed? We should point users to the docs to reduce support requests

This comment has been minimized.

Copy link
@sulkaharo

sulkaharo Jun 22, 2021

Member

No wait it changed places. If you change the string like this, you also need to commit a change to the localisation template that changes the strings accordingly.

,message: "Your Nightscout installation is readable by anyone who knows the web page URL."
,persistent: true
,seeAlso: 'disableWorldReadable'
};
ctx.adminnotifies.addNotify(message);
}
Expand Down

0 comments on commit 68f3f90

Please sign in to comment.