Skip to content

Commit 1e77517

Browse files
clemgbldfraxkenCopilot
authored
feat: new optional synchronous IO warning (#355)
Co-authored-by: Thomas.G <gentilhomme.thomas@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent c3016e0 commit 1e77517

64 files changed

Lines changed: 463 additions & 6 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

README.md

Lines changed: 4 additions & 1 deletion

docs/synchronous-io.md

Lines changed: 19 additions & 0 deletions

src/AstAnalyser.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@ export class AstAnalyser {
1919
* @param {SourceParser} [options.customParser]
2020
* @param {Array<object>} [options.customProbes]
2121
* @param {boolean} [options.skipDefaultProbes=false]
22+
* @param {boolean | Iterable<string>} [options.optionalWarnings=false]
2223
*/
2324
constructor(options = {}) {
2425
this.parser = options.customParser ?? new JsSourceParser();
2526
this.probesOptions = {
2627
customProbes: options.customProbes ?? [],
27-
skipDefaultProbes: options.skipDefaultProbes ?? false
28+
skipDefaultProbes: options.skipDefaultProbes ?? false,
29+
optionalWarnings: options.optionalWarnings ?? false
2830
};
2931
}
3032

src/ProbeRunner.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import isArrayExpression from "./probes/isArrayExpression.js";
1414
import isESMExport from "./probes/isESMExport.js";
1515
import isFetch from "./probes/isFetch.js";
1616
import isUnsafeCommand from "./probes/isUnsafeCommand.js";
17+
import isSyncIO from "./probes/isSyncIO.js";
1718

1819
/**
1920
* @typedef {import('./SourceFile.js').SourceFile} SourceFile
@@ -56,6 +57,14 @@ export class ProbeRunner {
5657
isUnsafeCommand
5758
];
5859

60+
/**
61+
*
62+
* @type {Record<string,Probe>}
63+
*/
64+
static Optionals = {
65+
"synchronous-io": isSyncIO
66+
};
67+
5968
/**
6069
*
6170
* @param {!SourceFile} sourceFile

src/SourceFile.js

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,36 @@ import * as trojan from "./obfuscators/trojan-source.js";
1111

1212
// CONSTANTS
1313
const kMaximumEncodedLiterals = 10;
14+
const kIdentifierOrMemberExps = [
15+
"crypto.createHash",
16+
"crypto.pbkdf2Sync",
17+
"crypto.scryptSync",
18+
"crypto.generateKeyPairSync",
19+
"fs.readFileSync",
20+
"fs.writeFileSync",
21+
"fs.appendFileSync",
22+
"fs.readSync",
23+
"fs.writeSync",
24+
"fs.readdirSync",
25+
"fs.statSync",
26+
"fs.mkdirSync",
27+
"fs.renameSync",
28+
"fs.unlinkSync",
29+
"fs.symlinkSync",
30+
"fs.openSync",
31+
"fs.fstatSync",
32+
"fs.linkSync",
33+
"fs.realpathSync",
34+
"child_process.execSync",
35+
"child_process.spawnSync",
36+
"child_process.execFileSync",
37+
"zlib.deflateSync",
38+
"zlib.inflateSync",
39+
"zlib.gzipSync",
40+
"zlib.gunzipSync",
41+
"zlib.brotliCompressSync",
42+
"zlib.brotliDecompressSync"
43+
];
1444

1545
export class SourceFile {
1646
inTryStatement = false;
@@ -24,15 +54,30 @@ export class SourceFile {
2454

2555
constructor(sourceCodeString, probesOptions = {}) {
2656
this.tracer = new VariableTracer()
27-
.enableDefaultTracing()
28-
.trace("crypto.createHash", {
29-
followConsecutiveAssignment: true, moduleName: "crypto"
30-
});
57+
.enableDefaultTracing();
58+
59+
kIdentifierOrMemberExps.forEach((identifierOrMemberExp) => this.tracer.trace(identifierOrMemberExp, {
60+
followConsecutiveAssignment: true,
61+
moduleName: identifierOrMemberExp.split(".")[0]
62+
}));
3163

3264
let probes = ProbeRunner.Defaults;
3365
if (Array.isArray(probesOptions.customProbes) && probesOptions.customProbes.length > 0) {
3466
probes = probesOptions.skipDefaultProbes === true ? probesOptions.customProbes : [...probes, ...probesOptions.customProbes];
3567
}
68+
69+
if (typeof probesOptions.optionalWarnings === "boolean") {
70+
if (probesOptions.optionalWarnings) {
71+
probes = [...probes, ...Object.values(ProbeRunner.Optionals)];
72+
}
73+
}
74+
else {
75+
const optionalProbes = Array.from(probesOptions.optionalWarnings ?? [])
76+
.flatMap((warning) => ProbeRunner.Optionals[warning] ?? []);
77+
78+
probes = [...probes, ...optionalProbes];
79+
}
80+
3681
this.probesRunner = new ProbeRunner(this, probes);
3782

3883
if (trojan.verify(sourceCodeString)) {

src/probes/isSyncIO.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Import Third-party Dependencies
2+
import { getCallExpressionIdentifier } from "@nodesecure/estree-ast-utils";
3+
4+
// Constants
5+
const kTracedNodeCoreModules = ["fs", "crypto", "child_process", "zlib"];
6+
7+
function validateNode(node, { tracer }) {
8+
const id = getCallExpressionIdentifier(node, { tracer });
9+
if (id === null || !kTracedNodeCoreModules.some((moduleName) => tracer.importedModules.has(moduleName))) {
10+
return [false];
11+
}
12+
13+
const data = tracer.getDataFromIdentifier(id);
14+
15+
return [data !== null && data.identifierOrMemberExpr.endsWith("Sync")];
16+
}
17+
18+
function main(node, { sourceFile }) {
19+
sourceFile.addWarning("synchronous-io", node.callee.name, node.loc);
20+
}
21+
22+
export default {
23+
name: "isSyncIO",
24+
validateNode,
25+
main,
26+
breakOnMatch: false
27+
};

src/warnings.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ export const warnings = Object.freeze({
5656
i18n: "sast_warnings.unsafe-command",
5757
severity: "Warning",
5858
experimental: true
59+
},
60+
"synchronous-io": {
61+
i18n: "sast_warnings.synchronous-io",
62+
severity: "Warning",
63+
experimental: true
5964
}
6065
});
6166

test/AstAnalyser.spec.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,14 @@ describe("AstAnalyser", () => {
698698
assert.strictEqual(FakeSourceParser.prototype.parse.mock.calls.length, 1);
699699
});
700700
});
701+
702+
describe("optional warnings", () => {
703+
it("should not crash when there is an unknown optional warning", () => {
704+
new AstAnalyser({
705+
optionalWarnings: ["unknown"]
706+
}).analyse("");
707+
});
708+
});
701709
});
702710

703711
let analyser = null;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
const { appendFileSync } = require("fs");
2+
3+
appendFileSync("./foo.txt");
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
const { brotliCompressSync } = require("zlib");
2+
3+
brotliCompressSync(Buffer.from("text","utf-8"));

0 commit comments

Comments
 (0)