Skip to content

Commit 50eb513

Browse files
committed
JS: Model Sails Action2 inputs as remote sources
1 parent f4773e9 commit 50eb513

13 files changed

Lines changed: 194 additions & 0 deletions

File tree

docs/codeql/reusables/supported-frameworks.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ and the CodeQL library pack ``codeql/javascript-all`` (`changelog <https://githu
191191
react native, HTML framework
192192
request, Network communicator
193193
restify, Server
194+
Sails.js Action2 controllers, Server
194195
sequelize, Database
195196
socket.io, Network communicator
196197
sqlite3, Database
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added support for treating declared `inputs` properties in Sails Action2 controller files as remote flow sources. This may improve results for security queries such as `js/path-injection`.

javascript/ql/lib/semmle/javascript/frameworks/HttpFrameworks.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ import semmle.javascript.frameworks.Micro
77
import semmle.javascript.frameworks.Restify
88
import semmle.javascript.frameworks.Connect
99
import semmle.javascript.frameworks.Fastify
10+
import semmle.javascript.frameworks.Sails
1011
import semmle.javascript.frameworks.Spife
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* Provides classes for working with [Sails](https://sailsjs.com/) applications.
3+
*/
4+
5+
import javascript
6+
import semmle.javascript.frameworks.HTTP
7+
private import DataFlow
8+
9+
/**
10+
* Provides classes for working with [Sails](https://sailsjs.com/) applications.
11+
*/
12+
module Sails {
13+
/**
14+
* A Sails Action2 action object exported from a module.
15+
*
16+
* For example:
17+
*
18+
* ```javascript
19+
* module.exports = {
20+
* inputs: { filename: { type: "string" } },
21+
* fn(inputs, exits) { ... }
22+
* };
23+
* ```
24+
*/
25+
private class Action2Action extends ObjectExpr {
26+
Action2Action() {
27+
exists(Module mod, DataFlow::FunctionNode fn |
28+
mod.getABulkExportedNode().getALocalSource().asExpr() = this and
29+
this.getFile().getRelativePath().regexpMatch("(^|.*/)api/controllers/.*") and
30+
this.getPropertyByName("inputs").getInit() instanceof ObjectExpr and
31+
fn = DataFlow::valueNode(this.getPropertyByName("fn").getInit()).getAFunctionValue()
32+
)
33+
}
34+
35+
/**
36+
* Gets the `fn` function that handles the action.
37+
*/
38+
DataFlow::FunctionNode getFunction() {
39+
result = DataFlow::valueNode(this.getPropertyByName("fn").getInit()).getAFunctionValue()
40+
}
41+
42+
/**
43+
* Gets the name of an input declared by this action.
44+
*/
45+
string getADeclaredInputName() {
46+
result = this.getPropertyByName("inputs").getInit().(ObjectExpr).getAProperty().getName()
47+
}
48+
}
49+
50+
/**
51+
* A Sails Action2 handler function.
52+
*/
53+
private class Action2RouteHandler extends Http::Servers::StandardRouteHandler,
54+
DataFlow::FunctionNode
55+
{
56+
Action2Action action;
57+
58+
Action2RouteHandler() { this = action.getFunction() }
59+
60+
/**
61+
* Gets the parameter of the action handler that contains the validated action inputs.
62+
*/
63+
DataFlow::ParameterNode getInputsParameter() { result = this.getParameter(0) }
64+
65+
/**
66+
* Gets the name of an input declared by this action.
67+
*/
68+
string getADeclaredInputName() { result = action.getADeclaredInputName() }
69+
}
70+
71+
/**
72+
* An access to a user-controlled Sails Action2 input.
73+
*/
74+
private class Action2InputAccess extends Http::RequestInputAccess {
75+
Action2RouteHandler rh;
76+
77+
Action2InputAccess() {
78+
this = rh.getInputsParameter().getAPropertyRead(rh.getADeclaredInputName())
79+
}
80+
81+
override Action2RouteHandler getRouteHandler() { result = rh }
82+
83+
override string getKind() { result = "parameter" }
84+
}
85+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
const action = {
2+
inputs: {
3+
name: { type: "string", required: true }
4+
},
5+
6+
fn(inputs, exits) {
7+
return exits.success(inputs.name);
8+
}
9+
};
10+
11+
module.exports = action;
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
module.exports = {
2+
inputs: {
3+
filename: { type: "string", required: true }
4+
},
5+
6+
async fn({ filename }, exits) {
7+
return exits.success(filename);
8+
}
9+
};
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
module.exports = {
2+
inputs: {
3+
id: { type: "string", regex: /^[0-9]+$/, required: true },
4+
filename: { type: "string", required: true }
5+
},
6+
7+
async fn(inputs, exits) {
8+
const id = inputs.id;
9+
const filename = inputs.filename;
10+
const notDeclared = inputs.notDeclared;
11+
12+
return exits.success({ id, filename, notDeclared });
13+
}
14+
};
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
module.exports = {
2+
inputs: {
3+
filename: { type: "string", required: true }
4+
},
5+
6+
async fn(inputs, exits) {
7+
return exits.success(inputs.filename);
8+
}
9+
};
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
module.exports = {
2+
inputs: {
3+
filename: { type: "string", required: true }
4+
},
5+
6+
fn(inputs, exits) {
7+
return exits.success(inputs.filename);
8+
}
9+
};
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
test_routeHandler
2+
| src/api/controllers/attachments/assigned-action.js:6:5:8:3 | (inputs ... e);\\n } |
3+
| src/api/controllers/attachments/destructured-action.js:6:11:8:3 | ({ file ... e);\\n } |
4+
| src/api/controllers/attachments/download-thumbnail.js:7:11:13:3 | (inputs ... });\\n } |
5+
test_requestInputAccess
6+
| src/api/controllers/attachments/assigned-action.js:7:26:7:36 | inputs.name | parameter |
7+
| src/api/controllers/attachments/destructured-action.js:6:14:6:21 | filename | parameter |
8+
| src/api/controllers/attachments/download-thumbnail.js:8:16:8:24 | inputs.id | parameter |
9+
| src/api/controllers/attachments/download-thumbnail.js:9:22:9:36 | inputs.filename | parameter |

0 commit comments

Comments
 (0)