Skip to content

Commit 2488138

Browse files
petersirkapetersirka
committed
Update prevention for injections in GM/IM.
1 parent 00bc73b commit 2488138

6 files changed

Lines changed: 19 additions & 37 deletions

File tree

‎changes.txt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
- added: hidden hack `res.noCompress = true` disables compilation of `.js` or `.css`
66
- added: `RESTBuilder` for creating REST requests (more in docs)
77
- added: new config item `allow-cache-snapshot` - to prevent cache when the framework is restarted
8-
- added: `String.prototype.escape_bash(command)` - to prevent bash injections
98

109
- updated: `SINGLETON(name, [def = {}])` about `def` argument
1110
- updated: `debug.js` adds timestamps

‎image.js‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ const spawn = child.spawn;
3333
const Fs = require('fs');
3434
const REGEXP_SVG = /(width=\"\d+\")+|(height=\"\d+\")+/g;
3535
const REGEXP_PATH = /\//g;
36+
const REGEXP_ARG = /\'/g;
3637

3738
var CACHE = {};
3839
var middlewares = {};
@@ -122,7 +123,7 @@ function Image(filename, useImageMagick, width, height) {
122123
this.width = width;
123124
this.height = height;
124125
this.builder = [];
125-
this.filename = type === 'string' ? filename.escape_bash() : null;
126+
this.filename = type === 'string' ? filename : null;
126127
this.currentStream = type === 'object' ? filename : null;
127128
this.isIM = useImageMagick == null ? F.config['default-image-converter'] === 'im' : useImageMagick;
128129
this.outputType = type === 'string' ? framework_utils.getExtension(filename) : 'jpg';
@@ -194,8 +195,7 @@ Image.prototype.save = function(filename, callback, writer) {
194195
if (typeof(filename) === 'function') {
195196
callback = filename;
196197
filename = null;
197-
} else if (filename)
198-
filename = filename.escape_bash();
198+
}
199199

200200
!self.builder.length && self.minify();
201201
filename = filename || self.filename || '';
@@ -228,9 +228,7 @@ Image.prototype.save = function(filename, callback, writer) {
228228
var writer = Fs.createWriteStream(filename + '_');
229229

230230
reader.pipe(middleware()).pipe(writer);
231-
writer.on('finish', function() {
232-
Fs.rename(filename + '_', filename, () => callback(null, true));
233-
});
231+
writer.on('finish', () => Fs.rename(filename + '_', filename, () => callback(null, true)));
234232
});
235233

236234
if (self.currentStream) {
@@ -267,7 +265,7 @@ Image.prototype.pipe = function(stream, type, options) {
267265
if (!type)
268266
type = self.outputType;
269267

270-
var cmd = spawn(self.isIM ? 'convert' : 'gm', self.arg(self.filename ? self.filename : '-', (type ? type + ':' : '') + '-'));
268+
var cmd = spawn(self.isIM ? 'convert' : 'gm', self.arg(self.filename ? wrap(self.filename) : '-', (type ? type + ':' : '') + '-'));
271269

272270
cmd.stderr.on('data', stream.emit.bind(stream, 'error'));
273271
cmd.stdout.on('data', stream.emit.bind(stream, 'data'));
@@ -305,7 +303,7 @@ Image.prototype.stream = function(type, writer) {
305303
if (!type)
306304
type = self.outputType;
307305

308-
var cmd = spawn(self.isIM ? 'convert' : 'gm', self.arg(self.filename ? self.filename : '-', (type ? type + ':' : '') + '-'));
306+
var cmd = spawn(self.isIM ? 'convert' : 'gm', self.arg(self.filename ? wrap(self.filename) : '-', (type ? type + ':' : '') + '-'));
309307
if (self.currentStream) {
310308
if (self.currentStream instanceof Buffer)
311309
cmd.stdin.end(self.currentStream);
@@ -333,7 +331,7 @@ Image.prototype.cmd = function(filenameFrom, filenameTo) {
333331
for (var i = 0; i < length; i++)
334332
cmd += (cmd ? ' ' : '') + self.builder[i].cmd;
335333

336-
return (self.isIM ? 'convert' : 'gm -convert') + ' "' + filenameFrom + '"' + ' ' + cmd + ' "' + filenameTo + '"';
334+
return (self.isIM ? 'convert' : 'gm -convert') + wrap(filenameFrom, true) + ' ' + cmd + wrap(filenameTo, true);
337335
};
338336

339337
Image.prototype.arg = function(first, last) {
@@ -371,7 +369,7 @@ Image.prototype.arg = function(first, last) {
371369
Image.prototype.identify = function(callback) {
372370
var self = this;
373371

374-
exec((self.isIM ? 'identify' : 'gm identify') + ' "' + self.filename + '"', function(err, stdout, stderr) {
372+
exec((self.isIM ? 'identify' : 'gm identify') + wrap(self.filename, true), function(err, stdout, stderr) {
375373

376374
if (err) {
377375
callback(err, null);
@@ -400,9 +398,9 @@ Image.prototype.push = function(key, value, priority, encode) {
400398

401399
if (value != null) {
402400
if (encode && typeof(value) === 'string')
403-
cmd += ' "' + value.escape_bash() + '"';
401+
cmd += wrap(value, true);
404402
else
405-
cmd += ' "' + value + '"';
403+
cmd += ' \'' + value + '\'';
406404
}
407405

408406
var obj = CACHE[cmd];
@@ -676,6 +674,13 @@ Image.prototype.command = function(key, value, priority, esc) {
676674
return this.push(key, value, priority || 10, esc);
677675
};
678676

677+
function wrap(command, empty) {
678+
var cmd = '';
679+
for (var i = 0, length = command.length; i < length; i++)
680+
cmd += command[i] === '\'' ? '\\' + command[i] : command[i];
681+
return (empty ? ' ' : '') + '\'' + cmd + '\'';
682+
}
683+
679684
exports.Image = Image;
680685
exports.Picture = Image;
681686

‎index.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ function Framework() {
467467

468468
this.id = null;
469469
this.version = 2200;
470-
this.version_header = '2.2.0-8';
470+
this.version_header = '2.2.0-9';
471471
this.version_node = process.version.toString().replace('v', '').replace(/\./g, '').parseFloat();
472472

473473
this.config = {

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@
7575
"name": "Peter Štolc",
7676
"email": "stolcp@gmail.com"
7777
}],
78-
"version": "2.2.0-8",
78+
"version": "2.2.0-9",
7979
"homepage": "http://www.totaljs.com",
8080
"bugs": {
8181
"url": "https://github.com/totaljs/framework/issues",

‎test/test-utils.js‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,6 @@ function prototypeString() {
9191
assert.ok('&lt;b&gt;total.js&lt;/b&gt;&amp;nbsp;'.decode() === '<b>total.js</b>&nbsp;', 'string.decode()');
9292
assert.ok(str.trim().replaceAt(5, ';') === 'total;js', 'string.replaceAt()');
9393

94-
str = '$("echo > 1")';
95-
assert.ok(str.escape_bash() === '(echo > 1)', 'string.escape_bash()');
96-
9794
str = ' A PeTer Širka Je krááály. ';
9895

9996
assert.ok(str.toSearch() === 'a peter sirka je krali', 'string.toSearch()');

‎utils.js‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3169,25 +3169,6 @@ String.prototype.fromUnicode = function() {
31693169
return unescape(str);
31703170
};
31713171

3172-
String.prototype.escape_bash = function() {
3173-
var output = '';
3174-
for (var i = 0, length = this.length; i < length; i++) {
3175-
var c = this[i];
3176-
switch (c) {
3177-
case '$':
3178-
case '"':
3179-
case '\'':
3180-
case '`':
3181-
case '\n':
3182-
continue;
3183-
default:
3184-
output += c;
3185-
continue;
3186-
}
3187-
}
3188-
return output;
3189-
};
3190-
31913172
String.prototype.sha1 = function(salt) {
31923173
var hash = crypto.createHash('sha1');
31933174
hash.update(this + (salt || ''), ENCODING);

0 commit comments

Comments
 (0)