From 26c882cda56ea679ef6311d729ae47cee0b1d1d3 Mon Sep 17 00:00:00 2001 From: Devin AI Date: Fri, 28 Aug 2026 16:03:20 +0000 Subject: [PATCH] bug: fix OS command injection in POST /create image handling (package install feature finding) --- routes/index.js | 30 +++++++++---- tests/create-command-injection.spec.js | 61 ++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 tests/create-command-injection.spec.js diff --git a/routes/index.js b/routes/index.js index 6b5455f03e4..68ec642203e 100644 --- a/routes/index.js +++ b/routes/index.js @@ -8,7 +8,7 @@ var ms = require('ms'); var streamBuffers = require('stream-buffers'); var readline = require('readline'); var moment = require('moment'); -var exec = require('child_process').exec; +var execFile = require('child_process').execFile; var validator = require('validator'); // zip-slip @@ -149,21 +149,35 @@ function parse(todo) { return t; } +// Characters outside this set (whitespace and every shell metacharacter) are never +// accepted in a URL handed to the image tooling. +var SAFE_URL_CHARS = /^[A-Za-z0-9._~:/?#@%+=-]+$/; +var MAX_URL_LENGTH = 2048; + +function isInspectableImageUrl(url) { + return typeof url === 'string' && url.length <= MAX_URL_LENGTH && + SAFE_URL_CHARS.test(url) && + validator.isURL(url, { protocols: ['http', 'https'], require_protocol: true }); +} + exports.create = function (req, res, next) { // console.log('req.body: ' + JSON.stringify(req.body)); var item = req.body.content; - var imgRegex = /\!\[alt text\]\((http.*)\s\".*/; + var imgRegex = /\!\[alt text\]\((http[^)\s]*)\s\".*/; if (typeof (item) == 'string' && item.match(imgRegex)) { var url = item.match(imgRegex)[1]; console.log('found img: ' + url); - exec('identify ' + url, function (err, stdout, stderr) { - console.log(err); - if (err !== null) { - console.log('Error (' + err + '):' + stderr); - } - }); + if (isInspectableImageUrl(url)) { + execFile('identify', [url], function (err, stdout, stderr) { + if (err !== null) { + console.log('Error (' + err + '):' + stderr); + } + }); + } else { + console.log('skipping identify for rejected img url'); + } } else { item = parse(item); diff --git a/tests/create-command-injection.spec.js b/tests/create-command-injection.spec.js new file mode 100644 index 00000000000..db89d05aca2 --- /dev/null +++ b/tests/create-command-injection.spec.js @@ -0,0 +1,61 @@ +var test = require('tap').test; +var path = require('path'); +var mongoose = require('mongoose'); +var childProcess = require('child_process'); + +var spawned = []; +childProcess.exec = function () { + spawned.push({ api: 'exec', args: Array.prototype.slice.call(arguments, 0, 1) }); +}; +childProcess.execFile = function (file, args, cb) { + spawned.push({ api: 'execFile', file: file, args: args }); + if (cb) cb(null, '', ''); +}; + +if (!mongoose.models.Todo) { + mongoose.model('Todo', new mongoose.Schema({ content: String, updated_at: Date })); +} +if (!mongoose.models.User) { + mongoose.model('User', new mongoose.Schema({ username: String, password: String })); +} +mongoose.Model.prototype.save = function (cb) { + cb(null, { content: Buffer.from(String(this.content || '')) }, 1); +}; + +var routes = require(path.join(__dirname, '..', 'routes', 'index.js')); + +function submit(content) { + spawned = []; + var res = { setHeader: function () {}, status: function () { return this; }, send: function () {} }; + routes.create({ body: { content: content } }, res, function (err) { throw err; }); + return spawned; +} + +var injections = [ + '![alt text](http://x/;id ")', + '![alt text](http://x/`id` ")', + '![alt text](http://x/$(id) ")', + '![alt text](http://x/|id ")', + '![alt text](http://x/ && touch /tmp/pwn ")', + '![alt text](http://x/\nid ")', + '![alt text](file:///etc/passwd ")', +]; + +test('POST /create never invokes a shell for todo image urls', function (t) { + injections.forEach(function (content) { + var calls = submit(content); + t.equal(calls.filter(function (c) { return c.api === 'exec'; }).length, 0, + 'no shell for ' + JSON.stringify(content)); + t.equal(calls.length, 0, 'no child process at all for ' + JSON.stringify(content)); + }); + t.end(); +}); + +test('POST /create still inspects a well-formed image url without a shell', function (t) { + var calls = submit('![alt text](http://example.com/snyk.png "logo")'); + t.equal(calls.length, 1, 'one child process'); + t.equal(calls[0].api, 'execFile', 'uses execFile'); + t.equal(calls[0].file, 'identify', 'runs identify'); + t.same(calls[0].args, ['http://example.com/snyk.png'], 'url passed as a single argument'); + t.end(); +});