hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 19:26:02 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #5117 from erik-krogh/parseForm

Browse Source 
Approved by asgerf
This commit is contained in:
CodeQL CI
2021-02-15 04:30:59 -08:00
committed by GitHub
parent 9b8d94d76e 74ce7369f8
commit b5143dbdb4
6 changed files with 219 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
javascript/change-notes/2021-02-09-form-parsers.md Normal file
View File

@@ -0,0 +1,7 @@
lgtm,codescanning
* Server side form parsing libraries are now recognized as source of remote user input.
Affected packages are
[multer](https://www.npmjs.com/package/multer),
[busboy](https://www.npmjs.com/package/busboy),
[formidable](https://www.npmjs.com/package/formidable), and
[multiparty](https://www.npmjs.com/package/formidable).

3
javascript/ql/src/javascript.qll
View File

@@ -85,10 +85,11 @@ import semmle.javascript.frameworks.Electron
import semmle.javascript.frameworks.EventEmitter
import semmle.javascript.frameworks.Files
import semmle.javascript.frameworks.Firebase
import semmle.javascript.frameworks.Immutable
import semmle.javascript.frameworks.FormParsers
import semmle.javascript.frameworks.jQuery
import semmle.javascript.frameworks.JWT
import semmle.javascript.frameworks.Handlebars
import semmle.javascript.frameworks.Immutable
import semmle.javascript.frameworks.LazyCache
import semmle.javascript.frameworks.LodashUnderscore
import semmle.javascript.frameworks.Logging

3
javascript/ql/src/semmle/javascript/frameworks/Express.qll
View File

@@ -509,8 +509,9 @@ module Express {
this = request.getAPropertyRead("cookies")
or
// `req.files`, treated the same as `req.body`.
// `express-fileupload` uses .files, and `multer` uses .files or .file
kind = "body" and
this = request.getAPropertyRead("files")
this = request.getAPropertyRead(["files", "file"])
)
or
kind = "body" and

63
javascript/ql/src/semmle/javascript/frameworks/FormParsers.qll Normal file
View File

@@ -0,0 +1,63 @@
/**
* Provides classes for modelling the server-side form/file parsing libraries.
*/
import javascript
/**
* A source of remote flow from the `Busboy` library.
*/
private class BusBoyRemoteFlow extends RemoteFlowSource {
BusBoyRemoteFlow() {
this =
API::moduleImport("busboy")
.getInstance()
.getMember("on")
.getParameter(1)
.getAParameter()
.getAnImmediateUse()
}
override string getSourceType() { result = "parsed user value from Busbuy" }
}
/**
* A source of remote flow from the `Formidable` library parsing a HTTP request.
*/
private class FormidableRemoteFlow extends RemoteFlowSource {
FormidableRemoteFlow() {
exists(API::Node formidable |
formidable = API::moduleImport("formidable").getReturn()
or
formidable = API::moduleImport("formidable").getMember("formidable").getReturn()
or
formidable =
API::moduleImport("formidable").getMember(["IncomingForm", "Formidable"]).getInstance()
|
this =
formidable.getMember("parse").getACall().getABoundCallbackParameter(1, any(int i | i > 0))
)
}
override string getSourceType() { result = "parsed user value from Formidable" }
}
/**
* A source of remote flow from the `Multiparty` library.
*/
private class MultipartyRemoteFlow extends RemoteFlowSource {
MultipartyRemoteFlow() {
exists(API::Node form | form = API::moduleImport("multiparty").getMember("Form").getInstance() |
exists(API::CallNode parse | parse = form.getMember("parse").getACall() |
this = parse.getParameter(1).getAParameter().getAnImmediateUse()
)
or
exists(API::CallNode on | on = form.getMember("on").getACall() |
on.getArgument(0).mayHaveStringValue(["part", "file", "field"]) and
this = on.getParameter(1).getAParameter().getAnImmediateUse()
)
)
}
override string getSourceType() { result = "parsed user value from Multiparty" }
}

82
javascript/ql/test/query-tests/Security/CWE-078/CommandInjection.expected
View File

@@ -88,6 +88,47 @@ nodes
| execSeries.js:18:34:18:40 | req.url |
| execSeries.js:19:12:19:16 | [cmd] |
| execSeries.js:19:13:19:15 | cmd |
| form-parsers.js:9:8:9:39 | "touch ... nalname |
| form-parsers.js:9:8:9:39 | "touch ... nalname |
| form-parsers.js:9:19:9:26 | req.file |
| form-parsers.js:9:19:9:26 | req.file |
| form-parsers.js:9:19:9:39 | req.fil ... nalname |
| form-parsers.js:13:3:13:11 | req.files |
| form-parsers.js:13:3:13:11 | req.files |
| form-parsers.js:13:21:13:24 | file |
| form-parsers.js:14:10:14:37 | "touch ... nalname |
| form-parsers.js:14:10:14:37 | "touch ... nalname |
| form-parsers.js:14:21:14:24 | file |
| form-parsers.js:14:21:14:37 | file.originalname |
| form-parsers.js:24:48:24:55 | filename |
| form-parsers.js:24:48:24:55 | filename |
| form-parsers.js:25:10:25:28 | "touch " + filename |
| form-parsers.js:25:10:25:28 | "touch " + filename |
| form-parsers.js:25:21:25:28 | filename |
| form-parsers.js:35:25:35:30 | fields |
| form-parsers.js:35:25:35:30 | fields |
| form-parsers.js:36:10:36:31 | "touch ... ds.name |
| form-parsers.js:36:10:36:31 | "touch ... ds.name |
| form-parsers.js:36:21:36:26 | fields |
| form-parsers.js:36:21:36:31 | fields.name |
| form-parsers.js:40:26:40:31 | fields |
| form-parsers.js:40:26:40:31 | fields |
| form-parsers.js:41:10:41:31 | "touch ... ds.name |
| form-parsers.js:41:10:41:31 | "touch ... ds.name |
| form-parsers.js:41:21:41:26 | fields |
| form-parsers.js:41:21:41:31 | fields.name |
| form-parsers.js:52:34:52:39 | fields |
| form-parsers.js:52:34:52:39 | fields |
| form-parsers.js:53:10:53:31 | "touch ... ds.name |
| form-parsers.js:53:10:53:31 | "touch ... ds.name |
| form-parsers.js:53:21:53:26 | fields |
| form-parsers.js:53:21:53:31 | fields.name |
| form-parsers.js:58:30:58:33 | part |
| form-parsers.js:58:30:58:33 | part |
| form-parsers.js:59:10:59:33 | "touch ... ilename |
| form-parsers.js:59:10:59:33 | "touch ... ilename |
| form-parsers.js:59:21:59:24 | part |
| form-parsers.js:59:21:59:33 | part.filename |
| lib/subLib/index.js:7:32:7:35 | name |
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name |
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name |
@@ -222,6 +263,40 @@ edges
| execSeries.js:18:34:18:40 | req.url | execSeries.js:18:13:18:47 | require ... , true) |
| execSeries.js:19:12:19:16 | [cmd] | execSeries.js:13:19:13:26 | commands |
| execSeries.js:19:13:19:15 | cmd | execSeries.js:19:12:19:16 | [cmd] |
| form-parsers.js:9:19:9:26 | req.file | form-parsers.js:9:19:9:39 | req.fil ... nalname |
| form-parsers.js:9:19:9:26 | req.file | form-parsers.js:9:19:9:39 | req.fil ... nalname |
| form-parsers.js:9:19:9:39 | req.fil ... nalname | form-parsers.js:9:8:9:39 | "touch ... nalname |
| form-parsers.js:9:19:9:39 | req.fil ... nalname | form-parsers.js:9:8:9:39 | "touch ... nalname |
| form-parsers.js:13:3:13:11 | req.files | form-parsers.js:13:21:13:24 | file |
| form-parsers.js:13:3:13:11 | req.files | form-parsers.js:13:21:13:24 | file |
| form-parsers.js:13:21:13:24 | file | form-parsers.js:14:21:14:24 | file |
| form-parsers.js:14:21:14:24 | file | form-parsers.js:14:21:14:37 | file.originalname |
| form-parsers.js:14:21:14:37 | file.originalname | form-parsers.js:14:10:14:37 | "touch ... nalname |
| form-parsers.js:14:21:14:37 | file.originalname | form-parsers.js:14:10:14:37 | "touch ... nalname |
| form-parsers.js:24:48:24:55 | filename | form-parsers.js:25:21:25:28 | filename |
| form-parsers.js:24:48:24:55 | filename | form-parsers.js:25:21:25:28 | filename |
| form-parsers.js:25:21:25:28 | filename | form-parsers.js:25:10:25:28 | "touch " + filename |
| form-parsers.js:25:21:25:28 | filename | form-parsers.js:25:10:25:28 | "touch " + filename |
| form-parsers.js:35:25:35:30 | fields | form-parsers.js:36:21:36:26 | fields |
| form-parsers.js:35:25:35:30 | fields | form-parsers.js:36:21:36:26 | fields |
| form-parsers.js:36:21:36:26 | fields | form-parsers.js:36:21:36:31 | fields.name |
| form-parsers.js:36:21:36:31 | fields.name | form-parsers.js:36:10:36:31 | "touch ... ds.name |
| form-parsers.js:36:21:36:31 | fields.name | form-parsers.js:36:10:36:31 | "touch ... ds.name |
| form-parsers.js:40:26:40:31 | fields | form-parsers.js:41:21:41:26 | fields |
| form-parsers.js:40:26:40:31 | fields | form-parsers.js:41:21:41:26 | fields |
| form-parsers.js:41:21:41:26 | fields | form-parsers.js:41:21:41:31 | fields.name |
| form-parsers.js:41:21:41:31 | fields.name | form-parsers.js:41:10:41:31 | "touch ... ds.name |
| form-parsers.js:41:21:41:31 | fields.name | form-parsers.js:41:10:41:31 | "touch ... ds.name |
| form-parsers.js:52:34:52:39 | fields | form-parsers.js:53:21:53:26 | fields |
| form-parsers.js:52:34:52:39 | fields | form-parsers.js:53:21:53:26 | fields |
| form-parsers.js:53:21:53:26 | fields | form-parsers.js:53:21:53:31 | fields.name |
| form-parsers.js:53:21:53:31 | fields.name | form-parsers.js:53:10:53:31 | "touch ... ds.name |
| form-parsers.js:53:21:53:31 | fields.name | form-parsers.js:53:10:53:31 | "touch ... ds.name |
| form-parsers.js:58:30:58:33 | part | form-parsers.js:59:21:59:24 | part |
| form-parsers.js:58:30:58:33 | part | form-parsers.js:59:21:59:24 | part |
| form-parsers.js:59:21:59:24 | part | form-parsers.js:59:21:59:33 | part.filename |
| form-parsers.js:59:21:59:33 | part.filename | form-parsers.js:59:10:59:33 | "touch ... ilename |
| form-parsers.js:59:21:59:33 | part.filename | form-parsers.js:59:10:59:33 | "touch ... ilename |
| lib/subLib/index.js:7:32:7:35 | name | lib/subLib/index.js:8:22:8:25 | name |
| lib/subLib/index.js:8:22:8:25 | name | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name |
| lib/subLib/index.js:8:22:8:25 | name | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name |
@@ -293,6 +368,13 @@ edges
| exec-sh2.js:10:12:10:57 | cp.spaw ... ptions) | exec-sh2.js:14:25:14:31 | req.url | exec-sh2.js:10:40:10:46 | command | This command depends on $@. | exec-sh2.js:14:25:14:31 | req.url | a user-provided value |
| exec-sh.js:15:12:15:61 | cp.spaw ... ptions) | exec-sh.js:19:25:19:31 | req.url | exec-sh.js:15:44:15:50 | command | This command depends on $@. | exec-sh.js:19:25:19:31 | req.url | a user-provided value |
| execSeries.js:14:41:14:47 | command | execSeries.js:18:34:18:40 | req.url | execSeries.js:14:41:14:47 | command | This command depends on $@. | execSeries.js:18:34:18:40 | req.url | a user-provided value |
| form-parsers.js:9:8:9:39 | "touch ... nalname | form-parsers.js:9:19:9:26 | req.file | form-parsers.js:9:8:9:39 | "touch ... nalname | This command depends on $@. | form-parsers.js:9:19:9:26 | req.file | a user-provided value |
| form-parsers.js:14:10:14:37 | "touch ... nalname | form-parsers.js:13:3:13:11 | req.files | form-parsers.js:14:10:14:37 | "touch ... nalname | This command depends on $@. | form-parsers.js:13:3:13:11 | req.files | a user-provided value |
| form-parsers.js:25:10:25:28 | "touch " + filename | form-parsers.js:24:48:24:55 | filename | form-parsers.js:25:10:25:28 | "touch " + filename | This command depends on $@. | form-parsers.js:24:48:24:55 | filename | a user-provided value |
| form-parsers.js:36:10:36:31 | "touch ... ds.name | form-parsers.js:35:25:35:30 | fields | form-parsers.js:36:10:36:31 | "touch ... ds.name | This command depends on $@. | form-parsers.js:35:25:35:30 | fields | a user-provided value |
| form-parsers.js:41:10:41:31 | "touch ... ds.name | form-parsers.js:40:26:40:31 | fields | form-parsers.js:41:10:41:31 | "touch ... ds.name | This command depends on $@. | form-parsers.js:40:26:40:31 | fields | a user-provided value |
| form-parsers.js:53:10:53:31 | "touch ... ds.name | form-parsers.js:52:34:52:39 | fields | form-parsers.js:53:10:53:31 | "touch ... ds.name | This command depends on $@. | form-parsers.js:52:34:52:39 | fields | a user-provided value |
| form-parsers.js:59:10:59:33 | "touch ... ilename | form-parsers.js:58:30:58:33 | part | form-parsers.js:59:10:59:33 | "touch ... ilename | This command depends on $@. | form-parsers.js:58:30:58:33 | part | a user-provided value |
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | child_process-test.js:85:37:85:54 | req.query.fileName | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | This command depends on $@. | child_process-test.js:85:37:85:54 | req.query.fileName | a user-provided value |
| other.js:7:33:7:35 | cmd | other.js:5:25:5:31 | req.url | other.js:7:33:7:35 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
| other.js:8:28:8:30 | cmd | other.js:5:25:5:31 | req.url | other.js:8:28:8:30 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |

63
javascript/ql/test/query-tests/Security/CWE-078/form-parsers.js Normal file
View File

@@ -0,0 +1,63 @@
var express = require('express');
var multer = require('multer');
var upload = multer({ dest: 'uploads/' });
var app = express();
var exec = require("child_process").exec;
app.post('/profile', upload.single('avatar'), function (req, res, next) {
exec("touch " + req.file.originalname); // NOT OK
});
app.post('/photos/upload', upload.array('photos', 12), function (req, res, next) {
req.files.forEach(file => {
exec("touch " + file.originalname); // NOT OK
})
});
var http = require('http');
var Busboy = require('busboy');
http.createServer(function (req, res) {
var busboy = new Busboy({ headers: req.headers });
busboy.on('file', function (fieldname, file, filename, encoding, mimetype) {
exec("touch " + filename); // NOT OK
});
req.pipe(busboy);
}).listen(8000);
const formidable = require('formidable');
app.post('/api/upload', (req, res, next) => {
let form = formidable({ multiples: true });
form.parse(req, (err, fields, files) => {
exec("touch " + fields.name); // NOT OK
});
let form2 = new formidable.IncomingForm();
form2.parse(req, (err, fields, files) => {
exec("touch " + fields.name); // NOT OK
});
});
var multiparty = require('multiparty');
var http = require('http');
http.createServer(function (req, res) {
// parse a file upload
var form = new multiparty.Form();
form.parse(req, function (err, fields, files) {
exec("touch " + fields.name); // NOT OK
});
var form2 = new multiparty.Form();
form2.on('part', function (part) { // / file / field
exec("touch " + part.filename); // NOT OK
});
form2.parse(req);
}).listen(8080);