Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Detect servlet configuration vulnerabilities with IAST #6300

Merged
merged 16 commits into from
Jan 9, 2024
Merged

Detect servlet configuration vulnerabilities with IAST #6300

merged 16 commits into from
Jan 9, 2024

Conversation

jandro996
Copy link
Member

@jandro996 jandro996 commented Nov 30, 2023 •
edited by jira bot
Loading

What Does This Do

Add 6 new vulnerabilities:

VERB_TAMPERING
ADMIN_CONSOLE_ACTIVE
DEFAULT_HTML_ESCAPE_INVALID
SESSION_TIMEOUT
DIRECTORY_LISTING_LEAK
INSECURE_JSP_LAYOUT

These vulnerabilities rely on the web.xml analysis or checking the jsp files location

We instrument the javax.servlet.ServletContext#getRealPath to be able to check the web.xml and the jsp files using the instrumentation context to call the module only once per context

Motivation

Innovation week project

Additional Notes

As we run it once per application there is no overheadController check

This PR aims to cover tomcat server

Jira ticket: APPSEC-17025

@pr-commenter
Copy link

pr-commenter bot commented Nov 30, 2023 •
edited
Loading

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master alejandro.gonzalez/IW_application_vulns
git_commit_date 1704706030 1704709056
git_commit_sha 5bf1b2f 83ef086
release_version 1.28.0-SNAPSHOT~5bf1b2f68e 1.28.0-SNAPSHOT~83ef086db0
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1704711674 1704711674
ci_job_id 402603453 402603453
ci_pipeline_id 26025162 26025162
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
module Agent Agent
parent None None
variant iast iast

Summary

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

Startup time reports for petclinic 
gantt
    title petclinic - global startup overhead: candidate=1.28.0-SNAPSHOT~83ef086db0, baseline=1.28.0-SNAPSHOT~5bf1b2f68e

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.05 s) : 0, 1049852
Total [baseline] (9.338 s) : 0, 9338211
Agent [candidate] (1.052 s) : 0, 1052361
Total [candidate] (9.397 s) : 0, 9396935
section appsec
Agent [baseline] (1.144 s) : 0, 1144476
Total [baseline] (9.435 s) : 0, 9435408
Agent [candidate] (1.148 s) : 0, 1147791
Total [candidate] (9.444 s) : 0, 9444306
section iast
Agent [baseline] (1.172 s) : 0, 1171597
Total [baseline] (9.6 s) : 0, 9600207
Agent [candidate] (1.18 s) : 0, 1180238
Total [candidate] (9.587 s) : 0, 9586705
section profiling
Agent [baseline] (1.278 s) : 0, 1277696
Total [baseline] (9.66 s) : 0, 9659580
Agent [candidate] (1.28 s) : 0, 1279937
Total [candidate] (9.659 s) : 0, 9659180
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.05 s -
Agent appsec 1.144 s 94.624 ms (9.0%)
Agent iast 1.172 s 121.745 ms (11.6%)
Agent profiling 1.278 s 227.843 ms (21.7%)
Total tracing 9.338 s -
Total appsec 9.435 s 97.197 ms (1.0%)
Total iast 9.6 s 261.996 ms (2.8%)
Total profiling 9.66 s 321.369 ms (3.4%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.052 s -
Agent appsec 1.148 s 95.43 ms (9.1%)
Agent iast 1.18 s 127.877 ms (12.2%)
Agent profiling 1.28 s 227.576 ms (21.6%)
Total tracing 9.397 s -
Total appsec 9.444 s 47.371 ms (0.5%)
Total iast 9.587 s 189.77 ms (2.0%)
Total profiling 9.659 s 262.245 ms (2.8%) 
gantt
    title petclinic - break down per module: candidate=1.28.0-SNAPSHOT~83ef086db0, baseline=1.28.0-SNAPSHOT~5bf1b2f68e

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (649.633 ms) : 0, 649633
BytebuddyAgent [candidate] (650.802 ms) : 0, 650802
GlobalTracer [baseline] (307.337 ms) : 0, 307337
GlobalTracer [candidate] (308.519 ms) : 0, 308519
AppSec [baseline] (50.719 ms) : 0, 50719
AppSec [candidate] (50.866 ms) : 0, 50866
Remote Config [baseline] (671.42 µs) : 0, 671
Remote Config [candidate] (667.868 µs) : 0, 668
Telemetry [baseline] (7.203 ms) : 0, 7203
Telemetry [candidate] (7.183 ms) : 0, 7183
section appsec
BytebuddyAgent [baseline] (648.238 ms) : 0, 648238
BytebuddyAgent [candidate] (650.096 ms) : 0, 650096
GlobalTracer [baseline] (306.465 ms) : 0, 306465
GlobalTracer [candidate] (307.546 ms) : 0, 307546
AppSec [baseline] (148.075 ms) : 0, 148075
AppSec [candidate] (148.424 ms) : 0, 148424
Remote Config [baseline] (634.817 µs) : 0, 635
Remote Config [candidate] (641.854 µs) : 0, 642
Telemetry [baseline] (6.906 ms) : 0, 6906
Telemetry [candidate] (6.873 ms) : 0, 6873
section iast
BytebuddyAgent [baseline] (771.923 ms) : 0, 771923
BytebuddyAgent [candidate] (777.657 ms) : 0, 777657
GlobalTracer [baseline] (285.826 ms) : 0, 285826
GlobalTracer [candidate] (287.759 ms) : 0, 287759
AppSec [baseline] (54.687 ms) : 0, 54687
AppSec [candidate] (51.912 ms) : 0, 51912
Remote Config [baseline] (567.678 µs) : 0, 568
Remote Config [candidate] (583.491 µs) : 0, 583
Telemetry [baseline] (6.502 ms) : 0, 6502
Telemetry [candidate] (6.569 ms) : 0, 6569
IAST [baseline] (17.772 ms) : 0, 17772
IAST [candidate] (21.123 ms) : 0, 21123
section profiling
ProfilingAgent [baseline] (124.508 ms) : 0, 124508
ProfilingAgent [candidate] (123.92 ms) : 0, 123920
BytebuddyAgent [baseline] (662.885 ms) : 0, 662885
BytebuddyAgent [candidate] (664.899 ms) : 0, 664899
GlobalTracer [baseline] (376.354 ms) : 0, 376354
GlobalTracer [candidate] (376.946 ms) : 0, 376946
AppSec [baseline] (51.171 ms) : 0, 51171
AppSec [candidate] (51.34 ms) : 0, 51340
Remote Config [baseline] (1.009 ms) : 0, 1009
Remote Config [candidate] (1.017 ms) : 0, 1017
Telemetry [baseline] (7.313 ms) : 0, 7313
Telemetry [candidate] (7.275 ms) : 0, 7275
Profiling [baseline] (124.534 ms) : 0, 124534
Profiling [candidate] (123.945 ms) : 0, 123945
Loading
Startup time reports for insecure-bank 
gantt
    title insecure-bank - global startup overhead: candidate=1.28.0-SNAPSHOT~83ef086db0, baseline=1.28.0-SNAPSHOT~5bf1b2f68e

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.056 s) : 0, 1055728
Total [baseline] (8.719 s) : 0, 8719405
Agent [candidate] (1.058 s) : 0, 1057969
Total [candidate] (8.78 s) : 0, 8780433
section iast
Agent [baseline] (1.178 s) : 0, 1177689
Total [baseline] (9.313 s) : 0, 9312679
Agent [candidate] (1.168 s) : 0, 1168480
Total [candidate] (9.266 s) : 0, 9265866
section iast_TELEMETRY_OFF
Agent [baseline] (1.16 s) : 0, 1159957
Total [baseline] (9.273 s) : 0, 9272577
Agent [candidate] (1.166 s) : 0, 1166066
Total [candidate] (9.253 s) : 0, 9253467
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.056 s -
Agent iast 1.178 s 121.961 ms (11.6%)
Agent iast_TELEMETRY_OFF 1.16 s 104.229 ms (9.9%)
Total tracing 8.719 s -
Total iast 9.313 s 593.274 ms (6.8%)
Total iast_TELEMETRY_OFF 9.273 s 553.172 ms (6.3%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.058 s -
Agent iast 1.168 s 110.511 ms (10.4%)
Agent iast_TELEMETRY_OFF 1.166 s 108.097 ms (10.2%)
Total tracing 8.78 s -
Total iast 9.266 s 485.432 ms (5.5%)
Total iast_TELEMETRY_OFF 9.253 s 473.034 ms (5.4%) 
gantt
    title insecure-bank - break down per module: candidate=1.28.0-SNAPSHOT~83ef086db0, baseline=1.28.0-SNAPSHOT~5bf1b2f68e

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (654.324 ms) : 0, 654324
BytebuddyAgent [candidate] (654.897 ms) : 0, 654897
GlobalTracer [baseline] (307.842 ms) : 0, 307842
GlobalTracer [candidate] (309.964 ms) : 0, 309964
AppSec [baseline] (50.974 ms) : 0, 50974
AppSec [candidate] (50.779 ms) : 0, 50779
Remote Config [baseline] (688.588 µs) : 0, 689
Remote Config [candidate] (670.426 µs) : 0, 670
Telemetry [baseline] (7.285 ms) : 0, 7285
Telemetry [candidate] (7.222 ms) : 0, 7222
section iast
BytebuddyAgent [baseline] (776.614 ms) : 0, 776614
BytebuddyAgent [candidate] (769.235 ms) : 0, 769235
GlobalTracer [baseline] (287.144 ms) : 0, 287144
GlobalTracer [candidate] (284.799 ms) : 0, 284799
AppSec [baseline] (53.928 ms) : 0, 53928
AppSec [candidate] (53.51 ms) : 0, 53510
IAST [baseline] (18.36 ms) : 0, 18360
IAST [candidate] (19.734 ms) : 0, 19734
Remote Config [baseline] (578.828 µs) : 0, 579
Remote Config [candidate] (561.671 µs) : 0, 562
Telemetry [baseline] (6.52 ms) : 0, 6520
Telemetry [candidate] (6.45 ms) : 0, 6450
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (761.692 ms) : 0, 761692
BytebuddyAgent [candidate] (765.531 ms) : 0, 765531
GlobalTracer [baseline] (284.898 ms) : 0, 284898
GlobalTracer [candidate] (286.445 ms) : 0, 286445
AppSec [baseline] (49.376 ms) : 0, 49376
AppSec [candidate] (49.188 ms) : 0, 49188
IAST [baseline] (22.733 ms) : 0, 22733
IAST [candidate] (23.524 ms) : 0, 23524
Remote Config [baseline] (571.748 µs) : 0, 572
Remote Config [candidate] (588.962 µs) : 0, 589
Telemetry [baseline] (6.467 ms) : 0, 6467
Telemetry [candidate] (6.476 ms) : 0, 6476
Loading

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2024-01-08T10:40:23 2024-01-08T10:57:00
git_branch master alejandro.gonzalez/IW_application_vulns
git_commit_date 1704706030 1704709056
git_commit_sha 5bf1b2f 83ef086
release_version 1.28.0-SNAPSHOT~5bf1b2f68e 1.28.0-SNAPSHOT~83ef086db0
start_time 2024-01-08T10:40:10 2024-01-08T10:56:47
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1704711674 1704711674
ci_job_id 402603453 402603453
ci_pipeline_id 26025162 26025162
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
variant iast iast

Summary

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

Request duration reports for petclinic 
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.28.0-SNAPSHOT~83ef086db0, baseline=1.28.0-SNAPSHOT~5bf1b2f68e
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.376 ms) : 1357, 1396
.   : milestone, 1376,
appsec (1.755 ms) : 1729, 1781
.   : milestone, 1755,
iast (1.548 ms) : 1524, 1572
.   : milestone, 1548,
profiling (1.516 ms) : 1490, 1541
.   : milestone, 1516,
tracing (1.531 ms) : 1506, 1556
.   : milestone, 1531,
section candidate
no_agent (1.373 ms) : 1353, 1392
.   : milestone, 1373,
appsec (1.779 ms) : 1754, 1804
.   : milestone, 1779,
iast (1.53 ms) : 1506, 1555
.   : milestone, 1530,
profiling (1.512 ms) : 1486, 1539
.   : milestone, 1512,
tracing (1.498 ms) : 1473, 1522
.   : milestone, 1498,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.376 ms [1.357 ms, 1.396 ms] -
appsec 1.755 ms [1.729 ms, 1.781 ms] 378.821 µs (27.5%)
iast 1.548 ms [1.524 ms, 1.572 ms] 171.897 µs (12.5%)
profiling 1.516 ms [1.49 ms, 1.541 ms] 139.317 µs (10.1%)
tracing 1.531 ms [1.506 ms, 1.556 ms] 154.616 µs (11.2%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.373 ms [1.353 ms, 1.392 ms] -
appsec 1.779 ms [1.754 ms, 1.804 ms] 406.695 µs (29.6%)
iast 1.53 ms [1.506 ms, 1.555 ms] 157.809 µs (11.5%)
profiling 1.512 ms [1.486 ms, 1.539 ms] 139.597 µs (10.2%)
tracing 1.498 ms [1.473 ms, 1.522 ms] 124.949 µs (9.1%)
Request duration reports for insecure-bank 
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.28.0-SNAPSHOT~83ef086db0, baseline=1.28.0-SNAPSHOT~5bf1b2f68e
    dateFormat X
    axisFormat %s
section baseline
no_agent (370.927 µs) : 351, 391
.   : milestone, 371,
iast (474.229 µs) : 454, 494
.   : milestone, 474,
iast_FULL (549.655 µs) : 529, 570
.   : milestone, 550,
iast_INACTIVE (453.175 µs) : 431, 475
.   : milestone, 453,
iast_TELEMETRY_OFF (476.653 µs) : 456, 497
.   : milestone, 477,
tracing (446.432 µs) : 425, 467
.   : milestone, 446,
section candidate
no_agent (367.648 µs) : 347, 388
.   : milestone, 368,
iast (485.56 µs) : 463, 508
.   : milestone, 486,
iast_FULL (560.762 µs) : 540, 581
.   : milestone, 561,
iast_INACTIVE (455.51 µs) : 434, 477
.   : milestone, 456,
iast_TELEMETRY_OFF (465.248 µs) : 445, 485
.   : milestone, 465,
tracing (449.24 µs) : 429, 469
.   : milestone, 449,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 370.927 µs [350.955 µs, 390.899 µs] -
iast 474.229 µs [454.057 µs, 494.4 µs] 103.302 µs (27.8%)
iast_FULL 549.655 µs [529.024 µs, 570.286 µs] 178.728 µs (48.2%)
iast_INACTIVE 453.175 µs [431.399 µs, 474.951 µs] 82.248 µs (22.2%)
iast_TELEMETRY_OFF 476.653 µs [455.931 µs, 497.376 µs] 105.727 µs (28.5%)
tracing 446.432 µs [425.392 µs, 467.471 µs] 75.505 µs (20.4%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 367.648 µs [347.118 µs, 388.179 µs] -
iast 485.56 µs [462.749 µs, 508.371 µs] 117.911 µs (32.1%)
iast_FULL 560.762 µs [540.378 µs, 581.145 µs] 193.113 µs (52.5%)
iast_INACTIVE 455.51 µs [434.141 µs, 476.879 µs] 87.862 µs (23.9%)
iast_TELEMETRY_OFF 465.248 µs [445.032 µs, 485.464 µs] 97.6 µs (26.5%)
tracing 449.24 µs [429.095 µs, 469.385 µs] 81.592 µs (22.2%)

@smola smola added the comp: asm iast Application Security Management (IAST) label Dec 1, 2023
@smola smola changed the title Innovation week - Application Vulnerabilities Detect servlet configuration vulnerabilities with IAST Dec 18, 2023
DDJavierSantos
DDJavierSantos reviewed Dec 20, 2023
View reviewed changes
dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/ApplicationModuleImpl.java

private static final String SESSION_TIMEOUT_END_TAG = "</session-timeout>";

private static final String SECURITY_CONSTRAINT_START_TAG = "<security-constraint>";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this work if there are spaces in the tag? "< security-constraint>" or "" or "< security-constraint >" what about uppercase tags? ""

Copy link
Member Author

@jandro996 jandro996 Jan 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that those cases can't happen:

  1. web.xml is case sensitive
  2. you are not allowed to open a tag with a space between the '<' and the first letter
  3. The available tags are defined in the schema so you can't have a valid uppercase tag that is defined lowercase in the schema, for instance <SECURITY-CONSTRAINT>

What is wrong right now is that we are not covering the <security-constraint > that is valid or the <security-constraint id="test">

Thanks for your comment!

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/ApplicationModuleImpl.java

checkInsecureJSPLayout(realPath, span);

String webXmlContent = webXmlContent(realPath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe should do
String webXmlContent = webXmlContent(realPath).toLowerCase();

since all tags are defined lowercase

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is not necessary, check my previous comment

@jandro996 jandro996 force-pushed the alejandro.gonzalez/IW_application_vulns branch from 333277e to bec0f08 Compare January 8, 2024 09:13
@jandro996 jandro996 marked this pull request as ready for review January 8, 2024 17:50
@jandro996 jandro996 requested review from a team as code owners January 8, 2024 17:50
@jandro996 jandro996 merged commit 2cc4653 into master Jan 9, 2024
73 checks passed
@jandro996 jandro996 deleted the alejandro.gonzalez/IW_application_vulns branch January 9, 2024 08:45
@github-actions github-actions bot added this to the 1.28.0 milestone Jan 9, 2024
@smola smola added the R&D label Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DDJavierSantos DDJavierSantos DDJavierSantos left review comments

@anderruiz anderruiz anderruiz approved these changes

@manuel-alvarez-alvarez manuel-alvarez-alvarez manuel-alvarez-alvarez approved these changes

@ValentinZakharov ValentinZakharov Awaiting requested review from ValentinZakharov

@smola smola Awaiting requested review from smola

@am312 am312 Awaiting requested review from am312 am312 was automatically assigned from DataDog/apm-java

@nayeem-kamal nayeem-kamal Awaiting requested review from nayeem-kamal nayeem-kamal was automatically assigned from DataDog/apm-java

Assignees
No one assigned
Labels
comp: asm iast Application Security Management (IAST) R&D
Projects
None yet
Milestone
1.28.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jandro996 @smola @anderruiz @manuel-alvarez-alvarez @DDJavierSantos