Skip to content

Fix stored XSS in public family/person registration (CVE-2023-24690)#7680

Merged
DawoudIO merged 2 commits intomasterfrom
fix/issue-6443-register-xss
Nov 30, 2025
Merged

Fix stored XSS in public family/person registration (CVE-2023-24690)#7680
DawoudIO merged 2 commits intomasterfrom
fix/issue-6443-register-xss

Conversation

@DawoudIO
Copy link
Contributor

@DawoudIO DawoudIO commented Nov 30, 2025

What Changed

Add input sanitization for public registration endpoints:

registerFamilyAPI:

  • Sanitize nested 'people' array with strip_tags() and htmlspecialchars()
  • Previously only top-level family fields were sanitized

registerPersonAPI:

  • Replace unsafe fromJSON() with explicit field setting
  • Add strip_tags() and htmlspecialchars() sanitization for all string inputs
  • Previously had no input sanitization at all

Fixes #6443

Type

  • ✨ Feature
  • 🐛 Bug fix
  • ♻️ Refactor
  • 🏗️ Build/Infrastructure
  • 🔒 Security

Testing

Screenshots

Security Check

  • Introduces new input validation
  • Modifies authentication/authorization
  • Affects data privacy/GDPR

Code Quality

  • Database: Propel ORM only, no raw SQL
  • No deprecated attributes (align, valign, nowrap, border, cellpadding, cellspacing, bgcolor)
  • Bootstrap CSS classes used
  • All CSS bundled via webpack

Pre-Merge

  • Tested locally
  • No new warnings
  • Build passes
  • Backward compatible (or migration documented)

@DawoudIO DawoudIO added this to the 6.3.0 milestone Nov 30, 2025
Copilot AI review requested due to automatic review settings November 30, 2025 00:17
@DawoudIO DawoudIO requested a review from a team as a code owner November 30, 2025 00:17
@DawoudIO DawoudIO requested review from DAcodedBEAT, MrClever, bigtigerku, grayeul and respencer and removed request for a team November 30, 2025 00:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes CVE-2023-24690, a stored XSS vulnerability in the public family and person registration endpoints. The fix adds comprehensive input sanitization using strip_tags() and htmlspecialchars() to prevent malicious HTML/JavaScript from being stored in the database.

Key Changes

  • Added sanitization for nested 'people' array in family registration (previously only top-level fields were sanitized)
  • Replaced unsafe fromJSON() method with explicit field-by-field setting in person registration
  • Applied consistent sanitization pattern across both endpoints: htmlspecialchars(trim(strip_tags($value)), ENT_QUOTES, 'UTF-8')

if (isset($personData['zip'])) {
$person->setZip($personData['zip']);
}

Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

The replacement of fromJSON() with explicit field setting may have removed support for birthday registration. The registerFamilyAPI handles birthday data (lines 87-94), and the previous fromJSON() implementation would have set any fields present in the JSON payload. If birthday data can be submitted to this endpoint, consider adding similar birthday handling:

if (isset($personData['birthday']) && !empty($personData['birthday'])) {
    $birthdayDate = DateTime::createFromFormat('m/d/Y', $personData['birthday']);
    if ($birthdayDate !== false) {
        $person->setBirthDay($birthdayDate->format('d'));
        $person->setBirthMonth($birthdayDate->format('m'));
        $person->setBirthYear($birthdayDate->format('Y'));
    }
}

Note: This assumes the birthday field should be sanitized like other string fields before parsing.

Suggested change
// Handle birthday field if present
if (isset($personData['birthday']) && !empty($personData['birthday'])) {
// Sanitize input
$birthdayString = \ChurchCRM\Utils\InputUtils::filterString($personData['birthday']);
$birthdayDate = DateTime::createFromFormat('m/d/Y', $birthdayString);
if ($birthdayDate !== false) {
$person->setBirthDay($birthdayDate->format('d'));
$person->setBirthMonth($birthdayDate->format('m'));
$person->setBirthYear($birthdayDate->format('Y'));
}
}

Copilot uses AI. Check for mistakes.
@DawoudIO DawoudIO merged commit e30be86 into master Nov 30, 2025
7 checks passed
@DawoudIO DawoudIO deleted the fix/issue-6443-register-xss branch November 30, 2025 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants