hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-07-29 23:00:09 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #22142 from theinfosecguy/js-sails-action2-inputs

Browse Source 
JS: Model Sails Action2 inputs as remote sources
This commit is contained in:
Asger F
2026-07-29 20:14:30 +02:00
committed by GitHub
parent 70b44c839c 7fc695f33b
commit cee85c1fed
13 changed files with 193 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
docs/codeql/reusables/supported-frameworks.rst
View File

@@ -191,6 +191,7 @@ and the CodeQL library pack ``codeql/javascript-all`` (`changelog <https://githu
react native, HTML framework
request, Network communicator
restify, Server
Sails.js Action2 controllers, Server
sequelize, Database
socket.io, Network communicator
sqlite3, Database

4
javascript/ql/lib/change-notes/2026-07-07-sails-action2-inputs.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* 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`.

1
javascript/ql/lib/semmle/javascript/frameworks/HttpFrameworks.qll
View File

@@ -7,4 +7,5 @@ import semmle.javascript.frameworks.Micro
import semmle.javascript.frameworks.Restify
import semmle.javascript.frameworks.Connect
import semmle.javascript.frameworks.Fastify
import semmle.javascript.frameworks.Sails
import semmle.javascript.frameworks.Spife

85
javascript/ql/lib/semmle/javascript/frameworks/Sails.qll Normal file
View File

@@ -0,0 +1,85 @@
/**
* Provides classes for working with [Sails](https://sailsjs.com/) applications.
*/
private import javascript
private import semmle.javascript.frameworks.HTTP
private import DataFlow
/**
* Provides classes for working with [Sails](https://sailsjs.com/) applications.
*/
module Sails {
/**
* A Sails Action2 action object exported from a module.
*
* For example:
*
* ```javascript
* module.exports = {
* inputs: { filename: { type: "string" } },
* fn(inputs, exits) { ... }
* };
* ```
*/
private class Action2Action extends ObjectExpr {
Action2Action() {
exists(Module mod, DataFlow::FunctionNode fn |
mod.getABulkExportedNode().getALocalSource().asExpr() = this and
this.getFile().getRelativePath().regexpMatch("(^|.*/)api/controllers/.*") and
this.getPropertyByName("inputs").getInit() instanceof ObjectExpr and
fn = DataFlow::valueNode(this.getPropertyByName("fn").getInit()).getAFunctionValue()
)
}
/**
* Gets the `fn` function that handles the action.
*/
DataFlow::FunctionNode getFunction() {
result = DataFlow::valueNode(this.getPropertyByName("fn").getInit()).getAFunctionValue()
}
/**
* Gets the name of an input declared by this action.
*/
string getADeclaredInputName() {
result = this.getPropertyByName("inputs").getInit().(ObjectExpr).getAProperty().getName()
}
}
/**
* A Sails Action2 handler function.
*/
private class Action2RouteHandler extends Http::Servers::StandardRouteHandler,
DataFlow::FunctionNode
{
Action2Action action;
Action2RouteHandler() { this = action.getFunction() }
/**
* Gets the parameter of the action handler that contains the validated action inputs.
*/
DataFlow::ParameterNode getInputsParameter() { result = this.getParameter(0) }
/**
* Gets the name of an input declared by this action.
*/
string getADeclaredInputName() { result = action.getADeclaredInputName() }
}
/**
* An access to a user-controlled Sails Action2 input.
*/
private class Action2InputAccess extends Http::RequestInputAccess {
Action2RouteHandler rh;
Action2InputAccess() {
this = rh.getInputsParameter().getAPropertyRead(rh.getADeclaredInputName())
}
override Action2RouteHandler getRouteHandler() { result = rh }
override string getKind() { result = "parameter" }
}
}

11
javascript/ql/test/library-tests/frameworks/Sails/src/api/controllers/attachments/assigned-action.js Normal file
View File

@@ -0,0 +1,11 @@
const action = {
inputs: {
name: { type: "string", required: true }
},
fn(inputs, exits) {
return exits.success(inputs.name);
}
};
module.exports = action;

9
javascript/ql/test/library-tests/frameworks/Sails/src/api/controllers/attachments/destructured-action.js Normal file
View File

@@ -0,0 +1,9 @@
module.exports = {
inputs: {
filename: { type: "string", required: true }
},
async fn({ filename }, exits) {
return exits.success(filename);
}
};

14
javascript/ql/test/library-tests/frameworks/Sails/src/api/controllers/attachments/download-thumbnail.js Normal file
View File

@@ -0,0 +1,14 @@
module.exports = {
inputs: {
id: { type: "string", regex: /^[0-9]+$/, required: true },
filename: { type: "string", required: true }
},
async fn(inputs, exits) {
const id = inputs.id;
const filename = inputs.filename;
const notDeclared = inputs.notDeclared;
return exits.success({ id, filename, notDeclared });
}
};

9
javascript/ql/test/library-tests/frameworks/Sails/src/api/helpers/read-thumbnail.js Normal file
View File

@@ -0,0 +1,9 @@
module.exports = {
inputs: {
filename: { type: "string", required: true }
},
async fn(inputs, exits) {
return exits.success(inputs.filename);
}
};

9
javascript/ql/test/library-tests/frameworks/Sails/src/lib/machine.js Normal file
View File

@@ -0,0 +1,9 @@
module.exports = {
inputs: {
filename: { type: "string", required: true }
},
fn(inputs, exits) {
return exits.success(inputs.filename);
}
};

9
javascript/ql/test/library-tests/frameworks/Sails/tests.expected Normal file
View File

@@ -0,0 +1,9 @@
test_routeHandler
| src/api/controllers/attachments/assigned-action.js:6:5:8:3 | (inputs ... e);\\n } |
| src/api/controllers/attachments/destructured-action.js:6:11:8:3 | ({ file ... e);\\n } |
| src/api/controllers/attachments/download-thumbnail.js:7:11:13:3 | (inputs ... });\\n } |
test_requestInputAccess
| src/api/controllers/attachments/assigned-action.js:7:26:7:36 | inputs.name | parameter |
| src/api/controllers/attachments/destructured-action.js:6:14:6:21 | filename | parameter |
| src/api/controllers/attachments/download-thumbnail.js:8:16:8:24 | inputs.id | parameter |
| src/api/controllers/attachments/download-thumbnail.js:9:22:9:36 | inputs.filename | parameter |

8
javascript/ql/test/library-tests/frameworks/Sails/tests.ql Normal file
View File

@@ -0,0 +1,8 @@
import javascript
import semmle.javascript.frameworks.Sails
query predicate test_routeHandler(Http::RouteHandler rh) { any() }
query predicate test_requestInputAccess(Http::RequestInputAccess ria, string kind) {
kind = ria.getKind()
}

8
javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected
View File

@@ -47,6 +47,7 @@
| TaintedPath.js:205:31:205:69 | path.re ... '), '') | TaintedPath.js:200:24:200:30 | req.url | TaintedPath.js:205:31:205:69 | path.re ... '), '') | This path depends on a $@. | TaintedPath.js:200:24:200:30 | req.url | user-provided value |
| TaintedPath.js:214:29:214:42 | improperEscape | TaintedPath.js:212:24:212:30 | req.url | TaintedPath.js:214:29:214:42 | improperEscape | This path depends on a $@. | TaintedPath.js:212:24:212:30 | req.url | user-provided value |
| TaintedPath.js:216:29:216:43 | improperEscape2 | TaintedPath.js:212:24:212:30 | req.url | TaintedPath.js:216:29:216:43 | improperEscape2 | This path depends on a $@. | TaintedPath.js:212:24:212:30 | req.url | user-provided value |
| api/controllers/attachments/download-thumbnail.js:18:42:18:49 | filePath | api/controllers/attachments/download-thumbnail.js:15:7:15:21 | inputs.filename | api/controllers/attachments/download-thumbnail.js:18:42:18:49 | filePath | This path depends on a $@. | api/controllers/attachments/download-thumbnail.js:15:7:15:21 | inputs.filename | user-provided value |
| examples/TaintedPath.js:10:29:10:43 | ROOT + filePath | examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:10:29:10:43 | ROOT + filePath | This path depends on a $@. | examples/TaintedPath.js:8:28:8:34 | req.url | user-provided value |
| execa.js:9:26:9:33 | filePath | execa.js:6:30:6:36 | req.url | execa.js:9:26:9:33 | filePath | This path depends on a $@. | execa.js:6:30:6:36 | req.url | user-provided value |
| execa.js:12:37:12:44 | filePath | execa.js:6:30:6:36 | req.url | execa.js:12:37:12:44 | filePath | This path depends on a $@. | execa.js:6:30:6:36 | req.url | user-provided value |
@@ -397,6 +398,9 @@ edges
| TaintedPath.js:215:9:215:23 | improperEscape2 | TaintedPath.js:216:29:216:43 | improperEscape2 | provenance | |
| TaintedPath.js:215:27:215:40 | unescape(path) | TaintedPath.js:215:9:215:23 | improperEscape2 | provenance | |
| TaintedPath.js:215:36:215:39 | path | TaintedPath.js:215:27:215:40 | unescape(path) | provenance | Config |
| api/controllers/attachments/download-thumbnail.js:11:9:11:16 | filePath | api/controllers/attachments/download-thumbnail.js:18:42:18:49 | filePath | provenance | |
| api/controllers/attachments/download-thumbnail.js:11:20:16:5 | path.jo ... e\\n ) | api/controllers/attachments/download-thumbnail.js:11:9:11:16 | filePath | provenance | |
| api/controllers/attachments/download-thumbnail.js:15:7:15:21 | inputs.filename | api/controllers/attachments/download-thumbnail.js:11:20:16:5 | path.jo ... e\\n ) | provenance | Config |
| examples/TaintedPath.js:8:7:8:14 | filePath | examples/TaintedPath.js:10:36:10:43 | filePath | provenance | |
| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | provenance | Config |
| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | provenance | Config |
@@ -950,6 +954,10 @@ nodes
| TaintedPath.js:215:27:215:40 | unescape(path) | semmle.label | unescape(path) |
| TaintedPath.js:215:36:215:39 | path | semmle.label | path |
| TaintedPath.js:216:29:216:43 | improperEscape2 | semmle.label | improperEscape2 |
| api/controllers/attachments/download-thumbnail.js:11:9:11:16 | filePath | semmle.label | filePath |
| api/controllers/attachments/download-thumbnail.js:11:20:16:5 | path.jo ... e\\n ) | semmle.label | path.jo ... e\\n ) |
| api/controllers/attachments/download-thumbnail.js:15:7:15:21 | inputs.filename | semmle.label | inputs.filename |
| api/controllers/attachments/download-thumbnail.js:18:42:18:49 | filePath | semmle.label | filePath |
| examples/TaintedPath.js:8:7:8:14 | filePath | semmle.label | filePath |
| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | semmle.label | url.par ... , true) |
| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | semmle.label | url.par ... ).query |

25
javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/api/controllers/attachments/download-thumbnail.js Normal file
View File

@@ -0,0 +1,25 @@
var fs = require("fs");
var path = require("path");
module.exports = {
inputs: {
id: { type: "string", regex: /^[0-9]+$/, required: true },
filename: { type: "string", required: true }
},
async fn(inputs, exits) {
var filePath = path.join(
"/srv/app/attachments",
"42",
"thumbnails",
inputs.filename // $ Source
);
return exits.success(fs.readFileSync(filePath)); // $ Alert
}
};
function localInputsAreNotRequests(inputs) {
var filePath = path.join("/srv/app/attachments", inputs.filename);
return fs.readFileSync(filePath);
}