Fix stored XSS in Calendar name (CVE-2023-24690)#7681
Conversation
Add strip_tags() sanitization when creating new calendars to prevent XSS payloads from being stored in the database. This is part of CVE-2023-24690 which covers multiple XSS vulnerabilities: - Calendar Name XSS (fixed here) - Group Name XSS (fixed in PR #7675) - Group Description XSS (fixed in PR #7675) Fixes #6444
There was a problem hiding this comment.
Pull request overview
This PR addresses part of CVE-2023-24690 by adding XSS prevention to calendar name creation. The fix applies strip_tags() sanitization when storing calendar names in the database, preventing malicious script injection through the calendar creation API endpoint. This is one of three XSS vulnerabilities in the CVE, with the other two (Group Name and Group Description) being addressed in PR #7675.
Key Changes:
- Added
strip_tags()sanitization to calendar name input in theNewCalendarAPI endpoint - Prevents stored XSS by stripping HTML/script tags before database insertion
- Complements React's automatic output escaping when displaying calendar names in the UI
src/api/routes/calendar/calendar.php
Outdated
| $input = $request->getParsedBody(); | ||
| $Calendar = new Calendar(); | ||
| $Calendar->setName($input['Name']); | ||
| $Calendar->setName(strip_tags($input['Name'])); |
There was a problem hiding this comment.
Missing input validation for the calendar name. If a malicious user submits HTML-only content (e.g., <script>alert('xss')</script>), strip_tags() will result in an empty string, allowing creation of calendars with blank names.
Consider adding validation to ensure a non-empty name:
$input = $request->getParsedBody();
if (empty($input['Name']) || empty(trim(strip_tags($input['Name'])))) {
throw new HttpBadRequestException($request, gettext('Calendar name is required'));
}
$Calendar = new Calendar();
$Calendar->setName(strip_tags($input['Name']));This prevents creation of calendars with empty or HTML-only names.
src/api/routes/calendar/calendar.php
Outdated
| $input = $request->getParsedBody(); | ||
| $Calendar = new Calendar(); | ||
| $Calendar->setName($input['Name']); | ||
| $Calendar->setName(strip_tags($input['Name'])); |
There was a problem hiding this comment.
Missing test coverage for the XSS security fix. While this fix addresses CVE-2023-24690, there's no test to verify that HTML/script tags are stripped from calendar names.
Consider adding a test in cypress/e2e/ui/admin/admin.calendar.spec.js similar to the existing XSS tests in other parts of the codebase (e.g., admin.group-xss.spec.js):
it("Should sanitize XSS payloads in calendar name", () => {
const xssPayload = '<script>alert("XSS")</script>SafeName';
cy.visit("v2/calendar");
cy.get("#newCalendarButton").click();
cy.get("#calendarName").type(xssPayload);
cy.get("#ForegroundColor").type("FA8072");
cy.get("#BackgroundColor").type("212F3D");
cy.get(".modal-footer > .pull-right").click();
// Verify calendar was created with sanitized name
cy.contains("SafeName").should("exist");
cy.contains("<script>").should("not.exist");
});This ensures the security fix works correctly and prevents regression.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
What Changed
Add strip_tags() sanitization when creating new calendars to prevent XSS payloads from being stored in the database.
This is part of CVE-2023-24690 which covers multiple XSS vulnerabilities:
Fixes #6444
Type
Testing
Screenshots
Security Check
Code Quality
Pre-Merge