Skip to content

Commit

Permalink
[BREAKING] Security: Disable JavaScript execution in Less.js
Browse files Browse the repository at this point in the history
Execution of JavaScript code located in *.less files is unexpected in
the context of OpenUI5 / SAPUI5 development and poses a security threat.

BREAKING CHANGE:
Parser option `javascriptEnabled` has been removed. JavaScript is always
disabled and cannot be enabled.
  • Loading branch information
matz3 committed Jan 29, 2021
1 parent 6db6f0a commit c0d3a85
Show file tree
Hide file tree
Showing 10 changed files with 144 additions and 75 deletions.
5 changes: 4 additions & 1 deletion README.md
Expand Up @@ -149,7 +149,10 @@ lib2
Type: `object`

Options for the [less](http://lesscss.org) parser (`less.Parser`).
**Note:** Default of `relativeUrls` option is changed from `false` to `true`.

**Note**
- Default of `relativeUrls` option is changed from `false` to `true`.
- Option `javascriptEnabled` has been removed. JavaScript is always disabled and cannot be enabled.

##### compiler

Expand Down
4 changes: 4 additions & 0 deletions lib/thirdparty/less/README.md
Expand Up @@ -3,3 +3,7 @@
This folder contains the `lib/less` sub-folder of [v1.6.3](https://github.com/less/less.js/tree/v1.6.3/lib/less) of the [less.js project](https://github.com/less/less.js) with commit [`ccd8ebbfdfa300b6e748e8d7c12e3dbb0efd8371`](https://github.com/less/less.js/commit/ccd8ebbfdfa300b6e748e8d7c12e3dbb0efd8371) applied on top of it to resolve https://github.com/SAP/less-openui5/issues/24.

The files `browser.js` and `rhino.js` have been removed, as they are not relevant for the Node.js implementation.

The file `tree/javascript.js` has been removed to disable JavaScript execution.

Modifications within the files are marked with `/* BEGIN MODIFICATION */` and `/* END MODIFICATION */` comments.
4 changes: 3 additions & 1 deletion lib/thirdparty/less/env.js
Expand Up @@ -14,7 +14,9 @@
'compress', // option - whether to compress
'processImports', // option - whether to process imports. if false then imports will not be imported
'syncImport', // option - whether to import synchronously
'javascriptEnabled',// option - whether JavaScript is enabled. if undefined, defaults to true
/* BEGIN MODIFICATION */
// Removed 'javascriptEnabled'
/* END MODIFICATION */
'mime', // browser only - mime type for sheet import
'useFileCache', // browser only - whether to use the per file session cache
'currentFileInfo' // information about the current file - for error reporting and importing and making urls relative etc.
Expand Down
6 changes: 5 additions & 1 deletion lib/thirdparty/less/functions.js
Expand Up @@ -213,7 +213,11 @@ tree.functions = {
}
},
e: function (str) {
return new(tree.Anonymous)(str instanceof tree.JavaScript ? str.evaluated : str);
/* BEGIN MODIFICATION */
// Removed handling of tree.JavaScript
return new(tree.Anonymous)(str);
/* END MODIFICATION */

},
escape: function (str) {
return new(tree.Anonymous)(encodeURI(str.value).replace(/=/g, "%3D").replace(/:/g, "%3A").replace(/#/g, "%23").replace(/;/g, "%3B").replace(/\(/g, "%28").replace(/\)/g, "%29"));
Expand Down
4 changes: 3 additions & 1 deletion lib/thirdparty/less/index.js
Expand Up @@ -112,7 +112,9 @@ require('./tree/mixin');
require('./tree/comment');
require('./tree/anonymous');
require('./tree/value');
require('./tree/javascript');
/* BEGIN MODIFICATION */
// Removed require('./tree/javascript');
/* END MODIFICATION */
require('./tree/assignment');
require('./tree/condition');
require('./tree/paren');
Expand Down
4 changes: 3 additions & 1 deletion lib/thirdparty/less/lessc_helper.js
Expand Up @@ -31,7 +31,9 @@ var lessc_helper = {
console.log(" -M, --depends Output a makefile import dependency list to stdout");
console.log(" --no-color Disable colorized output.");
console.log(" --no-ie-compat Disable IE compatibility checks.");
console.log(" --no-js Disable JavaScript in less files");
/* BEGIN MODIFICATION */
// Removed --no-js option
/* END MODIFICATION */
console.log(" -l, --lint Syntax check only (lint).");
console.log(" -s, --silent Suppress output of error messages.");
console.log(" --strict-imports Force evaluation of imports.");
Expand Down
18 changes: 7 additions & 11 deletions lib/thirdparty/less/parser.js
Expand Up @@ -993,27 +993,23 @@ less.Parser = function Parser(env) {
}
},

/* BEGIN MODIFICATION */
// Removed support for javascript

//
// JavaScript code to be evaluated
// JavaScript code (disabled)
//
// `window.location.href`
//
javascript: function () {
var str, j = i, e;
var j = i, e;

if (input.charAt(j) === '~') { j++; e = true; } // Escaped strings
if (input.charAt(j) !== '`') { return; }
if (env.javascriptEnabled !== undefined && !env.javascriptEnabled) {
error("You are using JavaScript, which has been disabled.");
}

if (e) { $char('~'); }

str = $re(/^`([^`]*)`/);
if (str) {
return new(tree.JavaScript)(str[1], i, e);
}
error("You are using JavaScript, which has been disabled.");
}
/* END MODIFICATION */
},

//
Expand Down
58 changes: 0 additions & 58 deletions lib/thirdparty/less/tree/javascript.js

This file was deleted.

8 changes: 7 additions & 1 deletion lib/thirdparty/less/tree/quoted.js
Expand Up @@ -22,7 +22,13 @@ tree.Quoted.prototype = {
eval: function (env) {
var that = this;
var value = this.value.replace(/`([^`]+)`/g, function (_, exp) {
return new(tree.JavaScript)(exp, that.index, true).eval(env).value;
/* BEGIN MODIFICATION */
// Removed support for javascript
const error = new Error("You are using JavaScript, which has been disabled.");
error.index = that.index;
error.type = "Syntax";
throw error;
/* END MODIFICATION */
}).replace(/@\{([\w-]+)\}/g, function (_, name) {
var v = new(tree.Variable)('@' + name, that.index, that.currentFileInfo).eval(env, true);
return (v instanceof tree.Quoted) ? v.value : v.toCSS();
Expand Down
108 changes: 108 additions & 0 deletions test/test.js
Expand Up @@ -419,6 +419,114 @@ describe("error handling", function() {
assert.ok(err);
});
});

it("should throw error when using inline JavaScript", function() {
const lessInput = `.rule {
@var: \`(function(){ return "Cat"; })()\`;
color: @var;
}`;
return new Builder().build({
lessInput
}).then(function(res) {
// no resolve
assert.ok(false, `Expected build to fail but finished with content:\n${res.css}`);
}, function(err) {
assert.equal(err.message, "You are using JavaScript, which has been disabled.");
assert.ok(err);
});
});

it("should throw error when using quoted inline JavaScript", function() {
const lessInput = `.rule {
@var: "\`(function(){ return 'Cat'; })()\`";
color: @var;
}`;
return new Builder().build({
lessInput
}).then(function(res) {
// no resolve
assert.ok(false, `Expected build to fail but finished with content:\n${res.css}`);
}, function(err) {
assert.equal(err.message, "You are using JavaScript, which has been disabled.");
assert.ok(err);
});
});

it("should throw error when using inline JavaScript with parser option javascriptEnabled: true", function() {
const lessInput = `.rule {
@var: \`(function(){ return "Cat"; })()\`;
color: @var;
}`;
return new Builder().build({
lessInput,
parser: {
javascriptEnabled: true
}
}).then(function(res) {
// no resolve
assert.ok(false, `Expected build to fail but finished with content:\n${res.css}`);
}, function(err) {
assert.equal(err.message, "You are using JavaScript, which has been disabled.");
assert.ok(err);
});
});

it("should throw error when using quoted inline JavaScript with parser option javascriptEnabled: true", function() {
const lessInput = `.rule {
@var: "\`(function(){ return 'Cat'; })()\`";
color: @var;
}`;
return new Builder().build({
lessInput,
parser: {
javascriptEnabled: true
}
}).then(function(res) {
// no resolve
assert.ok(false, `Expected build to fail but finished with content:\n${res.css}`);
}, function(err) {
assert.equal(err.message, "You are using JavaScript, which has been disabled.");
assert.ok(err);
});
});

it("should throw error when using inline JavaScript with parser option javascriptEnabled: false", function() {
const lessInput = `.rule {
@var: \`(function(){ return "Cat"; })()\`;
color: @var;
}`;
return new Builder().build({
lessInput,
parser: {
javascriptEnabled: false
}
}).then(function(res) {
// no resolve
assert.ok(false, `Expected build to fail but finished with content:\n${res.css}`);
}, function(err) {
assert.equal(err.message, "You are using JavaScript, which has been disabled.");
assert.ok(err);
});
});

it("should throw error when using quoted inline JavaScript with parser option javascriptEnabled: false", function() {
const lessInput = `.rule {
@var: "\`(function(){ return 'Cat'; })()\`";
color: @var;
}`;
return new Builder().build({
lessInput,
parser: {
javascriptEnabled: false
}
}).then(function(res) {
// no resolve
assert.ok(false, `Expected build to fail but finished with content:\n${res.css}`);
}, function(err) {
assert.equal(err.message, "You are using JavaScript, which has been disabled.");
assert.ok(err);
});
});
});

function assertLessToRtlCssEqual(filename) {
Expand Down

0 comments on commit c0d3a85

Please sign in to comment.