Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Xss in name field of custom reports #15007

Merged
merged 1 commit into from Apr 25, 2023
Merged

Conversation

aryaantony92
Copy link
Contributor

@aryaantony92 aryaantony92 commented Apr 21, 2023

Changes in this pull request

Resolves #

Additional info

WHAT

馃 Generated by Copilot at d6a57ca

This pull request fixes a XSS vulnerability in the custom report feature of the AdminBundle. It adds validation for the custom report name in the CustomReportController class and encodes/decodes the name in the panel.js file.

馃 Generated by Copilot at d6a57ca

CustomReportController is the gatekeeper of the name
No XSS can pass through its validation flame
deleteField must encode and decode with care
Or the panel.js will unleash the XSS nightmare

HOW

馃 Generated by Copilot at d6a57ca

  • Add validation checks for custom report name to prevent XSS vulnerability (link, link, link)
  • Extract validation logic to a helper method isValidConfigName in CustomReportController.php to avoid code duplication and improve readability (link)
  • Encode and decode custom report name when deleting a field in panel.js to prevent XSS vulnerability (link)

@github-actions
Copy link

github-actions bot commented Apr 21, 2023

Review Checklist

  • Target branch (10.5 for bug fixes, others 11.x)
  • Bug fix: check if files are affected that were moved to a bundle - create a PR there if applicable
  • Tests (if it's testable code, there should be a test for it - get help)
  • Docs (every functionality needs to be documented, see here)
  • Migration incl. install.sql (e.g. if the database schema changes, ...)
  • Upgrade notes (deprecations, important information, migration hints, ...)
  • Label
  • Milestone

@aryaantony92 aryaantony92 added this to the 10.5.21 milestone Apr 21, 2023
Copy link
Contributor

@martineiber martineiber left a comment

Choose a reason for hiding this comment

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

LGTM

@martineiber martineiber merged commit c36ef54 into 10.5 Apr 25, 2023
13 checks passed
@martineiber martineiber deleted the fix_custom_reports branch April 25, 2023 09:11
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.

None yet

2 participants