feat(probes): add unsafe-command
#327
Conversation
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
|
I don't worry much about false positive (we could still see in the real world if there is a lot or not). And CLI settings still propose to disable them (I think I will push to make experimental warning disable by default). |
|
Opened an issue in CLI: NodeSecure/cli#493 |
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
|
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
|
Now let's update doc and readme |
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
|
@fraxken I propose a |
Good for me |
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
|
Now I'll add |
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
| assert.equal(sastAnalysis.warnings().length, 0); | ||
| }); | ||
|
|
||
| // Exec |
There was a problem hiding this comment.
You'll realised that I copy paste the same cases. we could add both APIs (exec and spawn) for each cases. I just started with the simplest one :)
There was a problem hiding this comment.
Let me just know what do you prefer @fraxken I have no strong opinion.
There was a problem hiding this comment.
Hmm if tests are the same then you can probably do a for loop?
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
tony-go
changed the title
unsafe-command
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
and revert unrelated one Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
|
Do we also want to detect for synchronous methods? (such as execSync and spawnSync). |
yeah 🚀 that's a great idea |
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
Signed-off-by: Tony Gorez <gorez.tony@gmail.com>
| assert.equal(sastAnalysis.warnings().length, 0); | ||
| }); | ||
|
|
||
| // Exec |
There was a problem hiding this comment.
Hmm if tests are the same then you can probably do a for loop?
| return kUnsafeCommands.some((unsafeCommand) => command.includes(unsafeCommand)); | ||
| } | ||
|
|
||
| function isSpwanOrExec(name) { |
There was a problem hiding this comment.
| function isSpwanOrExec(name) { | |
| function isSpawnOrExec(name) { |
|
|
||
| let command = commandArg.value; | ||
| if (typeof command === "string" && isUnsafeCommand(command)) { | ||
| // Spwaned command arguments are filled into an Array |
There was a problem hiding this comment.
| // Spwaned command arguments are filled into an Array | |
| // Spawned command arguments are filled into an Array |
I would like to introduce
unsafe-commandprobe.I had the idea earlier in the day reading a book on macOS malware where authors try to detect if SIP is enabled (
csrutil).I noticed a bunch of commands that could be suspicious if passed in spawn/exec.
I imaged something where we could have a bunch of specific commands we could mark as suspicious.
My concerns is "What about false positives?" Maybe we would like to have a probe that could take a list of commands and add warnings only for these cases.