Skip to content

Security: Robust Fix for Reflected XSS and Missing CSRF Protection in Geocoding Prefix Reducer Tool#3983

Open
JoshuaProvoste wants to merge 1 commit intogoogle:masterfrom
JoshuaProvoste:fix/geodata-reducer-security
Open

Security: Robust Fix for Reflected XSS and Missing CSRF Protection in Geocoding Prefix Reducer Tool#3983
JoshuaProvoste wants to merge 1 commit intogoogle:masterfrom
JoshuaProvoste:fix/geodata-reducer-security

Conversation

@JoshuaProvoste
Copy link
Copy Markdown

Compliance with CONTRIBUTING.md:
Consistently with the project guidelines, this PR includes:

  • Unit Tests: Added CombineGeoDataServletTest.java to validate core security logic.
  • Online Demo Verification: While the official App Engine demo (libphonenumber-combine-geodata.appspot.com) is currently returning a 503 error, the vulnerability has been successfully reproduced and verified on localhost using the project's build tools.
  • Issue Tracker: Detailed report filed at Issue 498460232.
csrf_to_xss

poc.html


Description

This PR addresses two critical security vulnerabilities in the Geocoding Prefix Reducer tool (tools/java/data):

  1. Reflected XSS: User-provided inputs in the geodata parameter were being reflected directly into the HTML response without any sanitization. I have implemented a robust escapeHtml utility in CombineGeoData.java to ensure all outputs are properly encoded.
  2. Missing CSRF Protection: The /combine POST endpoint was vulnerable to Cross-Site Request Forgery. I have implemented session-based CSRF token generation and validation to secure this endpoint.

Technical Implementation Details

  • CombineGeoDataServlet.java:
    • Added doGet to generate and provide a unique CSRF token per session.
    • Updated doPost to strictly validate the presence and value of the csrf_token parameter.
  • CombineGeoData.java:
    • Added an escapeHtml method to handle <, >, &, ", and '.
    • Sanitized the output loop in run() to ensure data integrity while maintaining the prefix|description format.
  • Front-end Migration:
    • Converted index.html to index.jsp to dynamically inject the CSRF token into the form.
    • Updated web.xml to point to the new dynamic entry point.

Verification Performed

  • Reproduced Vulnerability: Confirmed the CSRF-to-XSS chain using a malicious POST form locally.
  • Validated Fix:
    • Verified that POST requests without a valid token are now rejected with 403 Forbidden.
    • Confirmed that script tags in geodata are rendered as harmless text (e.g., &lt;script&gt;).
    • Verified that standard geocoing data processing still functions as expected.
  • JUnit Tests: Added CombineGeoDataServletTest.java to automate the verification of the new security logic.

Checklist

  • Followed project guidelines for code hygiene.
  • Includes automated tests for the new logic.
  • No changes to metadata XML files.
  • Verified compatibility with Servlet 2.5 and GAE.

@JoshuaProvoste JoshuaProvoste requested a review from a team as a code owner April 2, 2026 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant