[Reachable] Fix CWE-79 (Cross-Site Scripting) vulnerability in src/main/java/org/owasp/benchmark/testcode/BenchmarkTest00492.java:55#718
Open
appsecai-app[bot] wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What we found
6a428858src/main/java/org/owasp/benchmark/testcode/BenchmarkTest00492.java:55Description: The servlet writes user-controlled input directly to an HTTP response writer without HTML encoding. The response content type is
text/html, making this a direct XSS sink. Attacker-supplied HTML or JavaScript in thebarparameter flows fromrequest.getParameterMap()throughThingFactory.createThing().doSomething()and intoresponse.getWriter().write()without any sanitization.Why this matters
Risk if not fixed: An attacker could inject malicious JavaScript that executes in victims' browsers. This enables session hijacking (stealing cookies), credential theft via fake login forms, malware distribution, or performing unauthorized actions on behalf of authenticated users.
Attack vector: Reflected XSS via HTTP request parameter
BenchmarkTest00492. An attacker crafts a malicious URL containing JavaScript payload and tricks a user into clicking it. The payload executes in the victim's browser with full access to the page's DOM and cookies.Mitigation bypass: The code explicitly sets
X-XSS-Protection: 0(line 54), disabling the browser's last-resort XSS filter. This removes a defense-in-depth layer that would otherwise block some reflected XSS attacks.Why we're changing it
Status: Confirmed vulnerability requiring remediation.
Vulnerability flow
request.getParameterMap().get('BenchmarkTest00492')[0]— fully attacker-controlled HTTP request parameterThingFactory.createThing().doSomething(param)— returns input unmodified (verified in Thing1.java and Thing2.java; no encoding applied)response.getWriter().write('Parameter value: ' + bar)— unsanitized string concatenated into text/html responseEvidence
baroriginates from request parameter with no intermediate encodingdoSomething()assigns parameter directly to return valuedoSomething()only guards null; StringBuilder conversion is a no-op with no HTML encodingtext/html(line 41), making the unescapedwrite()a direct XSS sinkHow we confirmed
Automated checks we ran:
LLM review of the diff:
Vulnerability Flow Diagram
%%{init: {'theme':'base','themeVariables':{'fontFamily':'ui-sans-serif, Inter, system-ui, sans-serif','primaryColor':'#EDE9FE','primaryTextColor':'#1A1A2E','primaryBorderColor':'#7C3AED','lineColor':'#5B21B6','secondaryColor':'#FEF3C7','tertiaryColor':'#DCFCE7'}}}%% flowchart TD A["Attacker sends<br/>script payload"] --> B["request.getParameterMap()<br/>BenchmarkTest00492"] B --> C["param = values[0]<br/>raw user input"] C --> D["ThingFactory.createThing()<br/>.doSomething(param)"] D --> E["bar = script payload<br/>unmodified"] E --> F{"Encoding applied?"} F -->|"Before fix"| G["writer.write()<br/>line 55 - no encoding"] G --> H["💥 Script Executes<br/>in Browser"] F -->|"After fix"| I["HtmlUtils.htmlEscape(bar)<br/>encodes ( ) and quotes"] I --> J["writer.write()<br/>safe encoded output"] J --> K["🛡️ Script Rendered<br/>as Plain Text"] style G fill:#FFE5E5,stroke:#F65A5A style H fill:#FEF3C7,stroke:#F59E0B style I fill:#DCFCE7,stroke:#16A34A style K fill:#DCFCE7,stroke:#16A34AVulnerable flow: src/main/java/org/owasp/benchmark/testcode/BenchmarkTest00492.java:55
Cross-Site Scripting
%%{init: {'theme':'base','themeVariables':{'fontFamily':'ui-sans-serif, Inter, system-ui, sans-serif','primaryColor':'#EDE9FE','primaryTextColor':'#1A1A2E','primaryBorderColor':'#7C3AED','lineColor':'#5B21B6','secondaryColor':'#FEF3C7','tertiaryColor':'#DCFCE7'}}}%% flowchart TD subgraph Vulnerable["❌ Vulnerable Flow"] direction LR A1["Attacker sends<br/>script payload"] --> A2["getParameterMap()<br/>BenchmarkTest00492"] A2 --> A3["param = values[0]<br/>raw user input"] A3 --> A4["ThingFactory<br/>doSomething(param)"] A4 --> A5["bar = script payload<br/>unmodified"] A5 --> A6["writer.write()<br/>line 55 - no encoding"] A6 --> A7["💥 Script Executes<br/>in Browser"] end Vulnerable ~~~ Fixed subgraph Fixed["✅ Fixed Flow"] direction LR B1["Attacker sends<br/>script payload"] --> B2["getParameterMap()<br/>BenchmarkTest00492"] B2 --> B3["param = values[0]<br/>raw user input"] B3 --> B4["ThingFactory<br/>doSomething(param)"] B4 --> B5["bar = script payload<br/>unmodified"] B5 --> B6["HtmlUtils.htmlEscape(bar)<br/>encodes metacharacters"] B6 --> B7["writer.write()<br/>safe encoded output"] B7 --> B8["🛡️ Script Rendered<br/>as Plain Text"] end style A6 fill:#FFE5E5,color:#1A1A2E style A7 fill:#ffa94d,color:#000 style B6 fill:#74c0fc,color:#000 style B8 fill:#DCFCE7,color:#000How we fixed it
Root cause
The servlet writes the
barvalue (derived from user-supplied request parameterBenchmarkTest00492) directly to the HTTP response writer via string concatenation without HTML encoding. Because the response content type istext/html, any HTML or JavaScript inbaris interpreted by the browser, enabling reflected XSS.Fix approach
Wrapping the output value with
org.springframework.web.util.HtmlUtils.htmlEscape(bar)converts HTML-special characters (<,>,&,",') to their safe entity equivalents before they reach the browser:This is the established
templates+helperstrategy already validated for this CWE-79/JVM bucket across sibling servlet files in the same remediation run. TheHtmlUtilsclass is already present in the Spring dependency (no new imports or dependencies required). The encoding is applied at the exact sink (thewrite()call), so the rest of the servlet logic is unaffected.Alternatives considered and rejected
Encode.forHtml()— Functionally equivalent but requires adding theowasp-java-encoderdependency, which is not present inpom.xml. Rejected in favor of the already-available Spring helper.<c:out>via JSP template — Architectural change requiring conversion of the raw writer approach to a template dispatch. Rejected as disproportionate for a single-line fix.Reusing existing pattern
Reusing existing pattern from sibling XSS remediations in the same run because
org.springframework.web.util.HtmlUtils.htmlEscape()is the canonicaltemplates+helperfix shape for CWE-79 in this JVM codebase and is already present in the Spring dependency.Files changed in this pull request (derived from the generated diff):
src/main/java/org/owasp/benchmark/testcode/BenchmarkTest00492.javaVulnerabilities Addressed
CWE-79
No Direct Response Writer
How we checked this fix
Automated checks we ran:
HtmlUtilsconfirmed present in Spring Framework (already in classpath)LLM review of the diff:
Verification script:
Runnable Verification Script (click to expand)
Save this script and run with
bash verify_xss_fix.sh:Reachability analysis
Reachability: Reachable
At least one remediated finding has a traced attacker-controlled path to the vulnerable sink.
Status: NO_PATH_FOUND (confidence 0.58, source: grounded call graph via Joern)
Exploitability score: 0/100
Grounded call graph built successfully, but no entry-point path reached the sink from a public HTTP endpoint. This indicates the vulnerable code may be part of a test harness or benchmark suite rather than production-facing code. However, the vulnerability is structurally present and confirmed: if the code is reachable via any HTTP request handler, the XSS is exploitable. The fix remains necessary to prevent accidental exposure if the code is later integrated into a live application.
How to verify
Manual browser test
src/main/java/org/owasp/benchmark/testcode/BenchmarkTest00492.java(line 55)BenchmarkTest00492request parameter:<script>alert('XSS')</script><script>alert('XSS')</script>), not trigger an alert<script>tagsAutomated test (optional)
If a unit test framework is available, verify that
HtmlUtils.htmlEscape()correctly encodes the payload:Before you merge
## How we fixed it: Fix addresses the root cause (output encoding at the sink), not just the symptom.## How we checked this fixto see which checks AppSecAI actually ran.## Reachability analysisand confirm whether this code is reachable from production HTTP endpoints or is test-only.## How to verifyin your environment before merging.org.springframework.web.util.HtmlUtilsis available in your Spring dependency version.X-XSS-Protection: 0header (line 54) to restore browser-side XSS filtering as a defense-in-depth layer.Learn more
This fix was generated by AppSecAI. Please review before merging.