dd-trace-js icon indicating copy to clipboard operation
dd-trace-js copied to clipboard

Exploit prevention Shell injection

Open uurien opened this issue 1 year ago • 2 comments

What does this PR do?

Modifies child_process instrumentation to add support for abortController and send file and fileArgs in separate properties, not only in command al together. With this new extra info, call to the waf and block the operation and request if needed.

Checklist

  • [ ] Unit tests.
  • [x] Add capability
  • [ ] Add System tests
  • [ ] Add integration test to test that app is not crashing

Additional Notes

uurien avatar Oct 16 '24 15:10 uurien

Overall package size

Self size: 7.89 MB Deduped: 64.91 MB No deduping: 65.25 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | @datadog/native-appsec | 8.2.1 | 19.18 MB | 19.19 MB | | @datadog/native-iast-taint-tracking | 3.2.0 | 13.9 MB | 13.91 MB | | @datadog/pprof | 5.4.1 | 9.76 MB | 10.13 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.5.0 | 2.51 MB | 2.65 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.0.1 | 1.06 MB | 1.46 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 826.22 kB | | msgpack-lite | 0.1.26 | 201.16 kB | 281.59 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 kB | | int64-buffer | 0.1.10 | 49.18 kB | 49.18 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | path-to-regexp | 0.1.10 | 6.38 kB | 6.38 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

github-actions[bot] avatar Oct 16 '24 15:10 github-actions[bot]

Benchmarks

Benchmark execution time: 2024-10-31 15:45:37

Comparing candidate commit 61898b362add14f89eed2523d236231e5ed267d8 in PR branch ugaitz/exploit-prevention-shi with baseline commit e7edfcffaf7871d33da810351ffb5ed3c887c132 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 260 metrics, 6 unstable metrics.

pr-commenter[bot] avatar Oct 16 '24 16:10 pr-commenter[bot]

I see some mesclun with naming. These changes are for detecting/blocking shell injection exploits, but files are named after command injection, as well as the RASP rule type

Is it something we can change to keep the consistency or I am missing something?

shell injection is a subset into command injection. For now, rule is called "command_injection", that is the reason that I used mostly command injection.

I used SHI for capability, and the tests. In the future when we support whole command injection, we will continue having only one command_injection.js file, and only one handler for it, but doing more things when shell: false. That is the reason why I decided to use command_injection for it, but the tests are only testing shell_injection, so the endpoints for the tests are SHI.

I'm not superhappy with the solution, but as you said, rule is called command-injection and the capability is called SHI... and currently it only works with shell.

uurien avatar Oct 24 '24 12:10 uurien

Codecov Report

Attention: Patch coverage is 96.47059% with 3 lines in your changes missing coverage. Please review.

Project coverage is 86.77%. Comparing base (fd0f570) to head (b47ec4b). Report is 45 commits behind head on master.

Files with missing lines Patch % Lines
...ages/datadog-instrumentations/src/child_process.js 95.08% 3 Missing :warning:
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4792       +/-   ##
===========================================
+ Coverage   68.58%   86.77%   +18.19%     
===========================================
  Files          12      151      +139     
  Lines         818     5383     +4565     
===========================================
+ Hits          561     4671     +4110     
- Misses        257      712      +455     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 31 '24 13:10 codecov[bot]