Skip to content

Commit 777b1e4

Browse files
committed
doc(warnings): add doc for perf
1 parent c47fe76 commit 777b1e4

File tree

6 files changed

+93
-41
lines changed

6 files changed

+93
-41
lines changed

README.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ Most of the time these hackers will try to hide the behaviour of their codes as
3636
- Highlight common attack patterns and API usages.
3737
- Capable to follow the usage of dangerous Node.js globals.
3838
- Detect obfuscated code and when possible the tool that has been used.
39+
- Detect potential performance issues.
3940

4041
## Getting Started
4142

@@ -106,7 +107,8 @@ type WarningName = "parsing-error"
106107
| "weak-crypto"
107108
| "unsafe-import"
108109
| "unsafe-command"
109-
| "shady-link";
110+
| "shady-link"
111+
| "perf";
110112

111113
declare const warnings: Record<WarningName, {
112114
i18n: string;
@@ -142,6 +144,7 @@ This section describe all the possible warnings returned by JSXRay. Click on the
142144
| [obfuscated-code](./docs/obfuscated-code.md) | ✔️ | There's a very high probability that the code is obfuscated. |
143145
| [weak-crypto](./docs/weak-crypto.md) || The code probably contains a weak crypto algorithm (md5, sha1...) |
144146
| [shady-link](./docs/shady-link.md) || The code contains shady/unsafe link |
147+
| [perf](./docs/perf.md) | ✔️ | The code probably contains performance issues. |
145148

146149
## Workspaces
147150

docs/perf.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# Perf
2+
3+
| Code | Severity | i18n | Experimental |
4+
| --- | --- | --- | :-: |
5+
| perf | `Warning` | `sast_warnings.perf` | ✔️ |
6+
7+
## Introduction
8+
9+
An **experimental** warning capable of detecting potential performance issues.
10+
11+
Such as of the synchronous API from Node.js core such as `fs` or `child_process` that can freeze the event loop.
12+
13+
[Node.js - Overview of Blocking vs Non-Blocking](https://nodejs.org/en/learn/asynchronous-work/overview-of-blocking-vs-non-blocking)
14+
15+
## Example
16+
17+
```js
18+
import fs from 'fs';
19+
20+
const data = fs.readFileSync('/foo.txt');
21+
```

src/SourceFile.js

Lines changed: 55 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -26,87 +26,113 @@ export class SourceFile {
2626
this.tracer = new VariableTracer()
2727
.enableDefaultTracing()
2828
.trace("crypto.createHash", {
29-
followConsecutiveAssignment: true,
29+
followConsecutiveAssignment: true,
3030
moduleName: "crypto"
3131
}).trace("crypto.pbkdf2Sync", {
32-
followConsecutiveAssignment: true ,
32+
followConsecutiveAssignment: true,
3333
moduleName: "crypto"
34-
}).trace("crypto.scryptSync", {
35-
followConsecutiveAssignment: true,
34+
})
35+
.trace("crypto.scryptSync", {
36+
followConsecutiveAssignment: true,
3637
moduleName: "crypto"
37-
}).trace("crypto.generateKeyPairSync", {
38+
})
39+
.trace("crypto.generateKeyPairSync", {
3840
followConsecutiveAssignment: true,
3941
moduleName: "crypto"
40-
}).trace("fs.readFileSync", {
42+
})
43+
.trace("fs.readFileSync", {
4144
followConsecutiveAssignment: true,
4245
moduleName: "fs"
43-
}).trace("fs.writeFileSync", {
46+
})
47+
.trace("fs.writeFileSync", {
4448
followConsecutiveAssignment: true,
4549
moduleName: "fs"
46-
}).trace("fs.appendFileSync", {
50+
})
51+
.trace("fs.appendFileSync", {
4752
followConsecutiveAssignment: true,
4853
moduleName: "fs"
49-
}).trace("fs.readSync", {
54+
})
55+
.trace("fs.readSync", {
5056
followConsecutiveAssignment: true,
5157
moduleName: "fs"
52-
}).trace("fs.writeSync", {
58+
})
59+
.trace("fs.writeSync", {
5360
followConsecutiveAssignment: true,
5461
moduleName: "fs"
55-
}).trace("fs.readdirSync", {
62+
})
63+
.trace("fs.readdirSync", {
5664
followConsecutiveAssignment: true,
5765
moduleName: "fs"
58-
}).trace("fs.statSync", {
66+
})
67+
.trace("fs.statSync", {
5968
followConsecutiveAssignment: true,
6069
moduleName: "fs"
61-
}).trace("fs.mkdirSync", {
70+
})
71+
.trace("fs.mkdirSync", {
6272
followConsecutiveAssignment: true,
6373
moduleName: "fs"
64-
}).trace("fs.renameSync", {
74+
})
75+
.trace("fs.renameSync", {
6576
followConsecutiveAssignment: true,
6677
moduleName: "fs"
67-
}).trace("fs.unlinkSync", {
78+
})
79+
.trace("fs.unlinkSync", {
6880
followConsecutiveAssignment: true,
6981
moduleName: "fs"
70-
}).trace("fs.symlinkSync", {
82+
})
83+
.trace("fs.symlinkSync", {
7184
followConsecutiveAssignment: true,
7285
moduleName: "fs"
73-
}).trace("fs.openSync", {
86+
})
87+
.trace("fs.openSync", {
7488
followConsecutiveAssignment: true,
7589
moduleName: "fs"
76-
}).trace("fs.fstatSync", {
90+
})
91+
.trace("fs.fstatSync", {
7792
followConsecutiveAssignment: true,
7893
moduleName: "fs"
79-
}).trace("fs.linkSync", {
94+
})
95+
.trace("fs.linkSync", {
8096
followConsecutiveAssignment: true,
8197
moduleName: "fs"
82-
}).trace("fs.realpathSync", {
98+
})
99+
.trace("fs.realpathSync", {
83100
followConsecutiveAssignment: true,
84101
moduleName: "fs"
85-
}).trace("child_process.execSync", {
102+
})
103+
.trace("child_process.execSync", {
86104
followConsecutiveAssignment: true,
87105
moduleName: "child_process"
88-
}).trace("child_process.spawnSync", {
106+
})
107+
.trace("child_process.spawnSync", {
89108
followConsecutiveAssignment: true,
90109
moduleName: "child_process"
91-
}).trace("child_process.execFileSync", {
110+
})
111+
.trace("child_process.execFileSync", {
92112
followConsecutiveAssignment: true,
93113
moduleName: "child_process"
94-
}).trace("zlib.deflateSync", {
114+
})
115+
.trace("zlib.deflateSync", {
95116
followConsecutiveAssignment: true,
96117
moduleName: "zlib"
97-
}).trace("zlib.inflateSync", {
118+
})
119+
.trace("zlib.inflateSync", {
98120
followConsecutiveAssignment: true,
99121
moduleName: "zlib"
100-
}).trace("zlib.gzipSync", {
122+
})
123+
.trace("zlib.gzipSync", {
101124
followConsecutiveAssignment: true,
102125
moduleName: "zlib"
103-
}).trace("zlib.gunzipSync", {
126+
})
127+
.trace("zlib.gunzipSync", {
104128
followConsecutiveAssignment: true,
105129
moduleName: "zlib"
106-
}).trace("zlib.brotliCompressSync", {
130+
})
131+
.trace("zlib.brotliCompressSync", {
107132
followConsecutiveAssignment: true,
108133
moduleName: "zlib"
109-
}).trace("zlib.brotliDecompressSync", {
134+
})
135+
.trace("zlib.brotliDecompressSync", {
110136
followConsecutiveAssignment: true,
111137
moduleName: "zlib"
112138
});

src/probes/isPerfConcern.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import { getCallExpressionIdentifier } from "@nodesecure/estree-ast-utils";
33

44
// Constants
5-
const kModules = ['fs', 'crypto', 'child_process', 'zlib'];
5+
const kModules = ["fs", "crypto", "child_process", "zlib"];
66

77
function validateNode(node, { tracer }) {
88
const id = getCallExpressionIdentifier(node, { tracer });
@@ -16,12 +16,12 @@ function validateNode(node, { tracer }) {
1616
}
1717

1818
function main(node, { sourceFile }) {
19-
sourceFile.addWarning("perf", node.callee.name, node.loc);
19+
sourceFile.addWarning("perf", node.callee.name, node.loc);
2020
}
2121

2222
export default {
2323
name: "isPerfConcern",
2424
validateNode,
2525
main,
26-
breakOnMatch: false
26+
breakOnMatch: false
2727
};

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+
perf: {
61+
i18n: "sast_warnings.perf",
62+
severity: "Warning",
63+
experimental: true
5964
}
6065
});
6166

test/probes/isPerfConcern.spec.js

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@ import { AstAnalyser } from "../../index.js";
99
const FIXTURE_URL = new URL("fixtures/perfConcern/", import.meta.url);
1010

1111
describe("isPerfConcern", () => {
12-
13-
test("it should report a warning in case of *Sync(...params)` usage", async() => {
12+
test("it should report a warning in case of *Sync(...params)` usage", async() => {
1413
const fixturesDir = new URL("directCallExpression/", FIXTURE_URL);
1514
const fixtureFiles = await fs.readdir(fixturesDir);
1615

@@ -40,34 +39,32 @@ test("it should report a warning in case of *Sync(...params)` usage", async() =>
4039
}
4140
});
4241

43-
44-
4542
test("it should NOT report a warning when relevant module is not imported", () => {
4643
const codes = [`
4744
const fs = {
4845
readFileSync() {}
4946
}
5047
fs.readFileSync('foo.txt');
5148
`,
52-
`
49+
`
5350
const crypto = {
5451
generateKeyPairSync() {}
5552
}
5653
crypto.generateKeyPairSync('foo.txt');
5754
`,
58-
`
55+
`
5956
const child_process = {
6057
execSync() {}
6158
}
6259
child_process.execSync('ls -la');
6360
`,
64-
`
61+
`
6562
const zlib = {
6663
gzipSync() {}
6764
}
6865
zlib.gzipSync(Buffer.from("text","utf-8"));
6966
`
70-
];
67+
];
7168
for (const code of codes) {
7269
const { warnings: outputWarnings } = new AstAnalyser().analyse(code);
7370
assert.strictEqual(outputWarnings.length, 0);

0 commit comments

Comments
 (0)