Skip to content

Security: Robust Fix for Reflected XSS and Missing CSRF Protection in Migrator Tool#3982

Open
JoshuaProvoste wants to merge 2 commits intogoogle:masterfrom
JoshuaProvoste:fix/migrator-csrf-xss
Open

Security: Robust Fix for Reflected XSS and Missing CSRF Protection in Migrator Tool#3982
JoshuaProvoste wants to merge 2 commits intogoogle:masterfrom
JoshuaProvoste:fix/migrator-csrf-xss

Conversation

@JoshuaProvoste
Copy link
Copy Markdown

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

  • Unit Tests: Added ServletMainTest.java to validate core security logic.
  • Online Demo Verification: Confirmed reproducibility on the official migrator demo.
  • Issue Tracker: Detailed report filed at Issue 498460231.
csrf_to_xss

poc_appspot.html


Description

This PR provides a robust and defensive fix for two security vulnerabilities identified in the Phone Number Migrator tool:

  1. Reflected XSS: Replaced all manual attribute reflections in index.jsp with JSTL <c:out>. This approach ensures all outputs are automatically HTML-encoded, protecting the application against malicious JavaScript injection (XSS).
  2. Missing CSRF Protection: Implemented a session-based CSRF token mechanism for the /migrate endpoint. The validation logic has been integrated into the ServletMain.java multipart processing loop to ensure early verification of incoming requests.

Issue Link (Google Issue Tracker): Fix for Reflected XSS and CSRF in Migrator Tool - Issue 498460231

Technical Implementation Details

  • index.jsp: Added JSTL taglib and refactored scriptlet-heavy blocks to use <c:choose>, <c:if>, and <c:out> for better data binding and security.
  • ServletMain.java:
    • doGet: Generates a unique CSRF token in the session and provides it to the JSP.
    • doPost: Validates the csrf_token field against the session-stored token before processing any migration logic.
  • Robustness: The fix covers more than 20 distinct reflection points and handles all phone-number-specific characters (+, -, etc.) correctly.

Verification Performed

  • Official Demo Reproduction: The vulnerability was successfully reproduced against the official web application at https://phone-number-migrator.uc.r.appspot.com/, confirming that the production environment is currently vulnerable to CSRF-to-XSS attacks.
  • Vulnerability confirmed: Reproduction achieved with a crafted poc.html triggering reflected script execution via cross-origin POST requests.
  • Security verified:
    • Re-tested with the same PoC: the CSRF token check now correctly rejects unauthorized requests (403 Forbidden).
    • Verified that malicious payloads are rendered as plain text due to JSTL's automatic escaping.
    • Confirmed that legitimate phone number migration still functions correctly.
  • JUnit Tests Added:
    • Created ServletMainTest.java to validate CSRF token generation and session handling logic.
    • Verified that tokens are correctly unique and persistent across requests.
    • Ensured that missing tokens correctly trigger validation failures.

Checklist

  • No unrelated refactorings included.
  • Correct branch naming conventions followed.
  • Follows existing project structure and dependency usage (JSTL 1.2).

@JoshuaProvoste JoshuaProvoste requested a review from a team as a code owner April 2, 2026 13:19
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