Skip to content

Commit

Permalink
Start abstracting some validations (OpenUserJS#1847)
Browse files Browse the repository at this point in the history
* Catch a few more instances of an invalid key check

Applies to OpenUserJS#944
  • Loading branch information
Martii authored Nov 20, 2021
1 parent 689775d commit b91b4b2
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 72 deletions.
98 changes: 26 additions & 72 deletions controllers/scriptStorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ var Discussion = require('../models/discussion').Discussion;
//--- Controller inclusions

//--- Library inclusions
// var scriptStorageLib = require('../libs/scriptStorage');
var scriptStorageLib = require('../libs/scriptStorage');

var patternHasSameOrigin = require('../libs/helpers').patternHasSameOrigin;
var patternMaybeSameOrigin = require('../libs/helpers').patternMaybeSameOrigin;
Expand Down Expand Up @@ -553,19 +553,6 @@ exports.sendScript = function (aReq, aRes, aNext) {

exports.getSource(aReq, function (aScript, aStream) {
let chunks = [];
let updateURL = null;
let updateUtf = null;

let matches = null;
let rAnyLocalMetaUrl = new RegExp(
'^' + patternHasSameOrigin +
'/(?:meta|install|src/scripts)/(.+?)/(.+?)\.(?:meta|user)\.js$'
);
let hasAlternateLocalUpdateURL = false;

let rSameOrigin = new RegExp(
'^' + patternHasSameOrigin
);

var lastModified = null;
var eTag = null;
Expand All @@ -579,53 +566,17 @@ exports.sendScript = function (aReq, aRes, aNext) {
return;
}

if (process.env.FORCE_BUSY_UPDATEURL_CHECK === 'true') {
// `@updateURL` must be exact here for OUJS hosted checks
// e.g. no `search`, no `hash`

updateURL = findMeta(aScript.meta, 'UserScript.updateURL.0.value');
if (updateURL) {

// Check for decoding error
try {
updateUtf = decodeURIComponent(updateURL);
} catch (aE) {
aRes.set('Warning', '199 ' + aReq.headers.host +
rfc2047.encode(' Invalid @updateURL'));
aRes.status(400).send(); // Bad request
return;
}

// Validate `author` and `name` (installNameBase) to this scripts meta only
let matches = updateUtf.match(rAnyLocalMetaUrl);
if (matches) {
if (cleanFilename(aScript.author, '').toLowerCase() +
'/' + cleanFilename(aScript.name, '') === matches[1].toLowerCase() + '/' + matches[2])
{
// Same script
} else {
hasAlternateLocalUpdateURL = true;
}
} else {
// Allow offsite checks
updateURL = new URL(updateURL);
if (rSameOrigin.test(updateURL.origin)) {
hasAlternateLocalUpdateURL = true;
}
}
} else {
if (!aScript.isLib) {
// Don't serve the script anywhere in this mode and if absent
hasAlternateLocalUpdateURL = true;
}
}

if (hasAlternateLocalUpdateURL) {
aRes.set('Warning', '199 ' + aReq.headers.host +
rfc2047.encode(' Invalid @updateURL in lockdown'));
aRes.status(404).send(); // Not found
return;
}
if (!scriptStorageLib.validKey(
aScript.author,
aScript.name,
aScript.isLib,
'updateURL',
findMeta(aScript.meta, 'UserScript.updateURL.0.value') // TODO: Simplify maybe
)) {
aRes.set('Warning', '199 ' + aReq.headers.host +
rfc2047.encode(' Invalid @updateURL'));
aRes.status(403).send(); // Forbidden
return;
}

hashSRI = aScript.hash
Expand Down Expand Up @@ -1221,6 +1172,7 @@ function isEqualKeyset(aSlaveKeyset, aMasterKeyset) {

exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) {
var isLib = !!findMeta(aMeta, 'UserLibrary');
var userName = aUser.name;
var scriptName = null;
var scriptDescription = null;
var thisName = null;
Expand Down Expand Up @@ -1692,20 +1644,22 @@ exports.storeScript = function (aUser, aMeta, aBuf, aUpdate, aCallback) {
aInnerCallback(null);
},
function (aInnerCallback) {
// `@updateURL` validations
var updateURL = null;

updateURL = findMeta(aMeta, 'UserScript.updateURL.0.value');
if (updateURL) {
if (!isFQUrl(updateURL)) {

// Not a web url... reject
// `@updateURL` validation

if (!scriptStorageLib.validKey(
userName,
scriptName,
isLib,
'updateURL',
findMeta(aMeta, 'UserScript.updateURL.0.value'))
) {
// Not a valid url... reject
aInnerCallback(new statusError({
message: '`@updateURL` not a web url',
code: 400
message: 'Author intervention required for `@updateURL`' +
(process.env.FORCE_BUSY_UPDATEURL_CHECK === 'true' ? ' in lockdown.' : '.'),
code: 403
}), null);
return;
}
}

aInnerCallback(null);
Expand Down
20 changes: 20 additions & 0 deletions controllers/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ var helpers = require('../libs/helpers');
var modelParser = require('../libs/modelParser');
var modelQuery = require('../libs/modelQuery');

var scriptStorageLib = require('../libs/scriptStorage');

var flagLib = require('../libs/flag');
var removeLib = require('../libs/remove');
var stats = require('../libs/stats');
Expand Down Expand Up @@ -2466,6 +2468,24 @@ exports.editScript = function (aReq, aRes, aNext) {

options.isOwner = authedUser && (authedUser._id == script._authorId
|| collaborators.indexOf(authedUser.name) > -1);

if (!options.isOwner && !options.isMod &&
!scriptStorageLib.validKey(
aScript.author,
aScript.name,
aScript.isLib,
'updateURL',
scriptStorage.findMeta(aScript.meta, 'UserScript.updateURL.0.value')
)
) {
statusCodePage(aReq, aRes, aNext, {
statusCode: 403,
statusMessage: 'Author intervention required for `@updateURL`' +
(process.env.FORCE_BUSY_UPDATEURL_CHECK === 'true' ? ' in lockdown.' : '.')
});
return;
}

modelParser.renderScript(script);
script.installNameSlug = installNameBase;

Expand Down
94 changes: 94 additions & 0 deletions libs/scriptStorage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
'use strict';

// Define some pseudo module globals
var isPro = require('../libs/debug').isPro;
var isDev = require('../libs/debug').isDev;
var isDbg = require('../libs/debug').isDbg;

//--- Library inclusions
var cleanFilename = require('../libs/helpers').cleanFilename;
var patternHasSameOrigin = require('../libs/helpers').patternHasSameOrigin;
var isFQUrl = require('../libs/helpers').isFQUrl;

//

// Determine validity of a Key
function validKey(aAuthorName, aScriptName, aIsLib, aKeyName, aKeyValue) {
var keyValue = null;
var keyValueUtf = null;
var matches = null;
var rAnyLocalMetaUrl = new RegExp(
'^' + patternHasSameOrigin +
'/(?:meta|install|src/scripts)/(.+?)/(.+?)\.(?:meta|user)\.js$'
);
var rSameOrigin = new RegExp(
'^' + patternHasSameOrigin
);

switch (aKeyName) {
case 'updateURL':
if (aIsLib) {
if (aKeyValue) {
return false;
}
} else {
if (process.env.FORCE_BUSY_UPDATEURL_CHECK === 'true') {
// `@updateURL` must be exact here for OUJS hosted checks and must exist
// e.g. no `search`, no `hash`

if (aKeyValue) {

// Check for decoding error
try {
keyValueUtf = decodeURIComponent(aKeyValue);
} catch (aE) {
return false;
}

// Validate `author` and `name` (installNameBase) to this scripts meta only
matches = keyValueUtf.match(rAnyLocalMetaUrl);
if (matches) {
if (cleanFilename(aAuthorName, '').toLowerCase() +
'/' + cleanFilename(aScriptName, '') === matches[1].toLowerCase() +
'/' + matches[2])
{
// Same script
// fallsthrough
} else {
return false;
}
} else {
// Allow offsite checks

if (!isFQUrl(aKeyValue)) {
return false;
}

keyValue = new URL(aKeyValue);
if (rSameOrigin.test(keyValue.origin)) {
return false;
}
}
} else {
if (!aIsLib) {
return false;
}
}
} else {
// Nominal tests

if (aKeyValue) {
if (!isFQUrl(aKeyValue)) {
return false;
}
}
}
}

break;
default:
}

return true;
}
exports.validKey = validKey;

0 comments on commit b91b4b2

Please sign in to comment.