Skip to content

fix: Session Fixation and CSV Injection vulnerabilities#670

Open
qiuluoli wants to merge 2 commits into
ryanhowdy:masterfrom
qiuluoli:fix/session-fixation-csv-injection
Open

fix: Session Fixation and CSV Injection vulnerabilities#670
qiuluoli wants to merge 2 commits into
ryanhowdy:masterfrom
qiuluoli:fix/session-fixation-csv-injection

Conversation

@qiuluoli
Copy link
Copy Markdown

Security Fixes

This PR fixes two security vulnerabilities:

1. Session Fixation (Fixes #537)

Problem: The app does not regenerate the session ID after successful authentication. An attacker can set a PHPSESSID before login, and the same session ID continues to be used after auth, allowing session fixation attacks.

Fix: Added session_regenerate_id(true) in the loginUser() function (familyconnections/inc/utils.php) after successful authentication but before setting session variables. This generates a new session ID and deletes the old one, preventing an attacker from using a pre-set session ID.

Reference: OWASP Session Fixation

2. CSV Injection (Fixes #539)

Problem: The address book export (Share > Address Book > Export) does not sanitize user-supplied data before writing to CSV. An attacker can inject formulas (e.g., =CMD(...)) in address fields, which execute when the CSV is opened in Excel or other spreadsheet applications.

Fix: Added a sanitization callback in displayExportSubmit() (familyconnections/addressbook.php) that prefixes cell values starting with dangerous characters (=, +, -, @, tab, CR) with a single quote, preventing formula execution while preserving the data.

Reference: CSV Injection - HackerOne

Testing

  • Session fixation: Login flow still works correctly; session ID is now regenerated on each login
  • CSV injection: Address fields with formula prefixes are properly escaped in export

@codacy-production
Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 654 complexity · 0 duplication

Metric Results
Complexity 654
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

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.

CSV Injection [$15] Session Fixation Vulnerability [$15]

1 participant