Skip to content
This repository was archived by the owner on Apr 15, 2020. It is now read-only.

Commit 4ef55a3

Browse files
roest01roest01
committed
resolve #340208 - Command injection in 'pdf-image', Severity:Medium (6.1) - fix #38 - solution for v2
1 parent d8c0dca commit 4ef55a3

3 files changed

Lines changed: 91 additions & 66 deletions

File tree

‎index.js‎

Lines changed: 59 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ var Promise = require("es6-promise").Promise;
44

55
var path = require("path");
66
var fs = require("fs");
7-
var util = require("util");
8-
var exec = require("child_process").exec;
7+
var spawn = require("child-process-promise").spawn;
98

109
function PDFImage(pdfFilePath, options) {
1110
if (!options) options = {};
@@ -23,10 +22,10 @@ function PDFImage(pdfFilePath, options) {
2322

2423
PDFImage.prototype = {
2524
constructGetInfoCommand: function () {
26-
return util.format(
27-
"pdfinfo \"%s\"",
28-
this.pdfFilePath
29-
);
25+
return {
26+
cmd: "pdfinfo",
27+
args: [this.pdfFilePath]
28+
};
3029
},
3130
parseGetInfoCommandOutput: function (output) {
3231
var info = {};
@@ -40,20 +39,12 @@ PDFImage.prototype = {
4039
getInfo: function () {
4140
var self = this;
4241
var getInfoCommand = this.constructGetInfoCommand();
43-
var promise = new Promise(function (resolve, reject) {
44-
exec(getInfoCommand, function (err, stdout, stderr) {
45-
if (err) {
46-
return reject({
47-
message: "Failed to get PDF'S information",
48-
error: err,
49-
stdout: stdout,
50-
stderr: stderr
51-
});
52-
}
53-
return resolve(self.parseGetInfoCommandOutput(stdout));
54-
});
42+
return new Promise(function (resolve, reject) {
43+
spawn(getInfoCommand.cmd, getInfoCommand.args, { capture: [ 'stdout', 'stderr' ]})
44+
.then(function (cmdResult) {
45+
resolve(self.parseGetInfoCommandOutput(cmdResult.stdout.toString()));
46+
}).catch(reject);
5547
});
56-
return promise;
5748
},
5849
numberOfPages: function () {
5950
return this.getInfo().then(function (info) {
@@ -84,46 +75,53 @@ PDFImage.prototype = {
8475
constructConvertCommandForPage: function (pageNumber) {
8576
var pdfFilePath = this.pdfFilePath;
8677
var outputImagePath = this.getOutputImagePathForPage(pageNumber);
87-
var convertOptionsString = this.constructConvertOptions();
88-
return util.format(
89-
"%s %s\"%s[%d]\" \"%s\"",
90-
this.useGM ? "gm convert" : "convert",
91-
convertOptionsString ? convertOptionsString + " " : "",
92-
pdfFilePath, pageNumber, outputImagePath
93-
);
78+
var convertOptions = this.constructConvertOptions();
79+
var args = [];
80+
if (convertOptions) args = convertOptions.slice();
81+
args.push(pdfFilePath+"["+pageNumber+"]");
82+
args.push(outputImagePath);
83+
84+
return {
85+
cmd: this.useGM ? "gm convert" : "convert",
86+
args: args
87+
};
9488
},
9589
constructCombineCommandForFile: function (imagePaths) {
96-
return util.format(
97-
"%s -append %s \"%s\"",
98-
this.useGM ? "gm convert" : "convert",
99-
imagePaths.join(' '),
100-
this.getOutputImagePathForFile()
101-
);
90+
var args = imagePaths.slice();
91+
args.push(this.getOutputImagePathForFile());
92+
args.unshift("-append");
93+
return {
94+
cmd: this.useGM ? "gm convert" : "convert",
95+
args: args
96+
};
10297
},
10398
constructConvertOptions: function () {
104-
return Object.keys(this.convertOptions).sort().map(function (optionName) {
99+
var convertOptions = [];
100+
Object.keys(this.convertOptions).sort().map(function (optionName) {
105101
if (this.convertOptions[optionName] !== null) {
106-
return optionName + " " + this.convertOptions[optionName];
102+
convertOptions.push(optionName);
103+
convertOptions.push(this.convertOptions[optionName]);
107104
} else {
108-
return optionName;
105+
convertOptions.push(optionName);
109106
}
110-
}, this).join(" ");
107+
}, this);
108+
return convertOptions;
111109
},
112110
combineImages: function(imagePaths) {
113111
var pdfImage = this;
114112
var combineCommand = pdfImage.constructCombineCommandForFile(imagePaths);
115113
return new Promise(function (resolve, reject) {
116-
exec(combineCommand, function (err, stdout, stderr) {
117-
if (err) {
118-
return reject({
114+
spawn(combineCommand.cmd, combineCommand.args, { capture: [ 'stdout', 'stderr' ]})
115+
.then(function () {
116+
spawn("rm", imagePaths); //cleanUp
117+
resolve(pdfImage.getOutputImagePathForFile());
118+
}).catch(function(error){
119+
reject({
119120
message: "Failed to combine images",
120-
error: err,
121-
stdout: stdout,
122-
stderr: stderr
121+
error: error.message,
122+
stdout: error.stdout,
123+
stderr: error.stderr
123124
});
124-
}
125-
exec("rm "+imagePaths.join(' ')); //cleanUp
126-
return resolve(pdfImage.getOutputImagePathForFile());
127125
});
128126
});
129127
},
@@ -167,16 +165,18 @@ PDFImage.prototype = {
167165

168166
var promise = new Promise(function (resolve, reject) {
169167
function convertPageToImage() {
170-
exec(convertCommand, function (err, stdout, stderr) {
171-
if (err) {
172-
return reject({
168+
return new Promise(function (resolve, reject) {
169+
spawn(convertCommand.cmd, convertCommand.args, { capture: [ 'stdout', 'stderr' ]})
170+
.then(function () {
171+
resolve(outputImagePath);
172+
}).catch(function(error){
173+
reject({
173174
message: "Failed to convert page to image",
174-
error: err,
175-
stdout: stdout,
176-
stderr: stderr
175+
error: error.message,
176+
stdout: error.stdout,
177+
stderr: error.stderr
177178
});
178-
}
179-
return resolve(outputImagePath);
179+
});
180180
});
181181
}
182182

@@ -194,7 +194,9 @@ PDFImage.prototype = {
194194

195195
if (imageNotExists) {
196196
// (1)
197-
convertPageToImage();
197+
convertPageToImage().then(function(result){
198+
resolve(result);
199+
}).catch(reject);
198200
return;
199201
}
200202

@@ -209,11 +211,10 @@ PDFImage.prototype = {
209211

210212
if (imageFileStat.mtime < pdfFileStat.mtime) {
211213
// (2)
212-
convertPageToImage();
213-
return;
214+
convertPageToImage().then(function(result){
215+
resolve(result);
216+
}).catch(reject);
214217
}
215-
216-
return resolve(outputImagePath);
217218
});
218219
});
219220
});

‎package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
},
1212
"homepage": "https://github.com/mooz/node-pdf-image#readme",
1313
"dependencies": {
14-
"es6-promise": "~4.2.4"
14+
"es6-promise": "~4.2.4",
15+
"child-process-promise": "^2.2.1"
1516
},
1617
"devDependencies": {
1718
"chai": "~4.1.2",

‎tests/test-main.js‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,19 +45,22 @@ describe("PDFImage", function () {
4545
});
4646

4747
it("should return correct convert command", function () {
48-
expect(pdfImage.constructConvertCommandForPage(1))
49-
.equal('convert "/tmp/test.pdf[1]" "/tmp/test-1.png"');
48+
var convertCommand = pdfImage.constructConvertCommandForPage(1);
49+
expect(convertCommand.cmd).equal("convert");
50+
expect(convertCommand.args.length).equal(2);
5051
});
5152

5253
it("should return correct convert command to combine images", function () {
53-
expect(pdfImage.constructCombineCommandForFile(['/tmp/test-0.png', '/tmp/test-1.png']))
54-
.equal('convert -append /tmp/test-0.png /tmp/test-1.png "/tmp/test.png"');
54+
var cmdConfig = pdfImage.constructCombineCommandForFile(['/tmp/test-0.png', '/tmp/test-1.png']);
55+
expect(cmdConfig.cmd).equal('convert');
56+
expect(cmdConfig.args.length).equal(4);
5557
});
5658

5759
it("should use gm when you ask it to", function () {
5860
pdfImage = new PDFImage(pdfPath, {graphicsMagick: true});
59-
expect(pdfImage.constructConvertCommandForPage(1))
60-
.equal('gm convert "/tmp/test.pdf[1]" "/tmp/test-1.png"');
61+
var cmdConfig = pdfImage.constructConvertCommandForPage(1);
62+
expect(cmdConfig.cmd).equal('gm convert');
63+
expect(cmdConfig.args.length).equal(2);
6164
});
6265

6366
// TODO: Do page updating test
@@ -148,7 +151,27 @@ describe("PDFImage", function () {
148151
"-density": 300,
149152
"-trim": null
150153
});
151-
expect(pdfImage.constructConvertOptions()).equal("-density 300 -trim");
154+
expect(pdfImage.constructConvertOptions()[0]).equal("-density 300");
155+
expect(pdfImage.constructConvertOptions()[1]).equal("-trim");
156+
});
157+
158+
it("should convert all PDF's pages with convertOptions", function () {
159+
return new Promise(function(resolve, reject){
160+
pdfImage.setConvertOptions({
161+
"-quality": 100,
162+
"-trim": null
163+
});
164+
165+
pdfImage.convertFile().then(function (images) {
166+
images.forEach(function(image){
167+
expect(fs.existsSync(image)).to.be.true;
168+
});
169+
generatedFiles = images;
170+
resolve();
171+
}).catch(function (error) {
172+
reject(error.message + " " + error.stderr);
173+
});
174+
})
152175
});
153176

154177
afterEach(function(done){

0 commit comments

Comments
 (0)