Mitigate CVE-2025-3573 #545
Conversation
Permits to have the translation of the messages
WalkthroughThe changes focus on improving internationalization and client-side validation security across the application. Error messages resulting from phone number validation are now consistently passed through translation functions before being displayed, ensuring localization throughout controllers and models. A new helper function, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Form (Client)
participant Server (Controller)
participant Model
participant Translator
User->>Form (Client): Submit form with phone number
Form (Client)->>Server (Controller): Send phone number for validation
Server (Controller)->>Model: Validate phone number
Model-->>Server (Controller): Return validation result (TRUE or error message)
alt Validation fails
Server (Controller)->>Translator: Translate error message (tr() or tr_raw())
Translator-->>Server (Controller): Localized error message
Server (Controller)-->>Form (Client): Return translated error message
Form (Client)-->>User: Display translated error message (with HTML escaped)
else Validation succeeds
Server (Controller)-->>Form (Client): Return success
Form (Client)-->>User: Proceed as normal
end
sequenceDiagram
participant Developer
participant Helper (i18n_helper)
participant check_translation.php
Developer->>Helper (i18n_helper): Call tr_no_op("Some string")
Helper (i18n_helper)-->>Developer: Return "Some string" unchanged
check_translation.php->>Helper (i18n_helper): Scan for tr_no_op calls
Helper (i18n_helper)-->>check_translation.php: Provide strings for translation extraction
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
application/helpers/kalkun_helper.php (1)
539-541: Fixed parameter name typo.The parameter name was corrected from a misspelled
$înputto the properly spelled$input. This improves code readability and maintainability while keeping the function's behavior unchanged.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
media/js/jquery-plugin/jquery.validate.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (21)
application/controllers/Kalkun.php(2 hunks)application/controllers/Messages.php(1 hunks)application/controllers/Phonebook.php(1 hunks)application/helpers/i18n_helper.php(1 hunks)application/helpers/kalkun_helper.php(2 hunks)application/language/french/kalkun_lang.php(2 hunks)application/models/Kalkun_model.php(1 hunks)application/models/User_model.php(1 hunks)application/plugins/server_alert/views/js_server_alert.php(1 hunks)application/plugins/sms_credit/views/js_users.php(1 hunks)application/plugins/soap/views/js_remote_access.php(2 hunks)application/plugins/stop_manager/controllers/Stop_manager.php(1 hunks)application/plugins/stop_manager/views/js_stop_manager.php(1 hunks)application/plugins/whitelist_number/views/js_whitelist_number.php(2 hunks)application/views/js_init/message/js_compose.php(1 hunks)application/views/js_init/users/js_add_user.php(1 hunks)application/views/main/phonebook/contact/add_contact.php(1 hunks)application/views/main/settings/password.php(1 hunks)application/views/main/settings/personal.php(1 hunks)docs/CREDITS(1 hunks)utils/check_translation.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
application/plugins/stop_manager/controllers/Stop_manager.php (1)
application/helpers/i18n_helper.php (1)
tr(25-28)
application/controllers/Phonebook.php (1)
application/helpers/i18n_helper.php (1)
tr(25-28)
application/models/User_model.php (1)
application/helpers/i18n_helper.php (1)
tr(25-28)
application/models/Kalkun_model.php (1)
application/helpers/i18n_helper.php (1)
tr(25-28)
application/helpers/kalkun_helper.php (1)
application/helpers/i18n_helper.php (1)
tr_no_op(91-94)
application/controllers/Kalkun.php (1)
application/helpers/i18n_helper.php (1)
tr_raw(40-43)
application/controllers/Messages.php (1)
application/helpers/i18n_helper.php (1)
tr(25-28)
application/views/main/phonebook/contact/add_contact.php (1)
application/helpers/i18n_helper.php (1)
tr_js(74-80)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test (ubuntu-latest, 5.6)
- GitHub Check: test (ubuntu-latest, 7.0)
- GitHub Check: check-code (ubuntu-latest, 8.2)
🔇 Additional comments (29)
application/plugins/server_alert/views/js_server_alert.php (1)
6-6: Security enhancement: Adding HTML escaping to prevent injection attacksThe addition of
escapeHtml: trueto the jQuery validation configuration is an excellent security improvement that prevents potential HTML injection attacks in validation error messages. This change directly addresses CVE-2025-3573 as mentioned in the PR objectives.application/plugins/sms_credit/views/js_users.php (1)
17-17: Security enhancement: Adding HTML escaping to prevent injection attacksThe addition of
escapeHtml: trueto the jQuery validation configuration is an excellent security improvement that prevents potential HTML injection attacks in validation error messages. This change directly addresses CVE-2025-3573 as mentioned in the PR objectives.application/helpers/i18n_helper.php (1)
81-94: Good addition of translation marker functionThe
tr_no_opfunction serves as a helpful utility for marking strings for translation without altering their content. This is particularly useful for the translation checking tools to identify strings that require translation, while preserving their original format.application/views/main/settings/personal.php (1)
6-6: Security enhancement: Adding HTML escaping to prevent injection attacksThe addition of
escapeHtml: trueto the jQuery validation configuration is an excellent security improvement that prevents potential HTML injection attacks in validation error messages. This change directly addresses CVE-2025-3573 as mentioned in the PR objectives.application/views/js_init/users/js_add_user.php (1)
7-7: Security enhancement: HTML escaping enabled for validation messagesAdding
escapeHtml: trueto the validation configuration properly mitigates CVE-2025-3573 by preventing potential XSS attacks through validation error messages. This is exactly the recommended fix from the jquery-validation project.docs/CREDITS (1)
13-13: Library version updated to address security vulnerabilityUpdating jQuery Validation to version 1.21.0 is appropriate as this version includes the security fix for CVE-2025-3573. This change aligns with the PR objective of mitigating the vulnerability.
Let's verify this is the appropriate version:
What version of jQuery Validation contains the fix for CVE-2025-3573?application/plugins/stop_manager/views/js_stop_manager.php (1)
50-50: Security enhancement: HTML escaping enabled for validation messagesAdding
escapeHtml: trueto the validation configuration properly mitigates CVE-2025-3573 by preventing potential XSS attacks through validation error messages. This is consistent with the changes made to other validation scripts in the application.application/views/main/settings/password.php (1)
6-6: Security enhancement: HTML escaping enabled for validation messagesAdding
escapeHtml: trueto the validation configuration properly mitigates CVE-2025-3573 by preventing potential XSS attacks through validation error messages. This is consistent with the changes made to other validation scripts in the application.application/views/js_init/message/js_compose.php (1)
196-196: Security enhancement: XSS protection added to validation messagesAdding
escapeHtml: trueto the jQuery validation configuration is a security best practice that prevents potential XSS attacks by escaping HTML characters in validation error messages. This change addresses CVE-2025-3573 as mentioned in the PR objectives.application/plugins/soap/views/js_remote_access.php (2)
13-13: Security enhancement: XSS protection added to validation messagesAdding
escapeHtml: trueto the jQuery validation configuration prevents potential XSS attacks by escaping HTML characters in validation error messages. This change helps mitigate CVE-2025-3573.
29-29: Security enhancement: XSS protection added to validation messagesSimilarly, adding
escapeHtml: trueto this form's validation configuration ensures consistent protection against XSS attacks across all forms in the application.application/language/french/kalkun_lang.php (2)
362-362: Fixed French translation typoThe French translation for "Please enter a name for your message" has been corrected from "Veuillz" to "Veuillez", fixing a typographical error.
382-382: Fixed French translation typoThe French translation for "Value is too short" has been corrected from "La valeur estr trop courte" to "La valeur est trop courte", fixing a typographical error.
application/plugins/whitelist_number/views/js_whitelist_number.php (2)
76-76: Security enhancement: XSS protection added to validation messagesAdding
escapeHtml: trueto the jQuery validation configuration helps prevent XSS attacks by escaping HTML characters in validation error messages. This change is consistent with the security improvements made throughout the application to address CVE-2025-3573.
98-98: Security enhancement: XSS protection added to validation messagesSimilarly, adding
escapeHtml: trueto this form's validation configuration ensures consistent protection against XSS attacks across all forms in the application.application/plugins/stop_manager/controllers/Stop_manager.php (1)
133-133: Enhanced security with error message escaping and translationThe change correctly wraps the phone number validation error message with the
tr()function, which provides two important benefits:
- It translates the error message based on the user's language settings
- It applies HTML entity encoding via
htmlentities()to prevent potential XSS attacksThis change is part of the security improvements to mitigate CVE-2025-3573 by ensuring error messages are properly escaped.
application/models/User_model.php (1)
171-171: Enhanced security with error message escaping and translationThe change correctly wraps the phone number validation error message with the
tr()function, which provides two important benefits:
- It translates the error message based on the user's language settings
- It applies HTML entity encoding via
htmlentities()to prevent potential XSS attacksThis change is part of the security improvements to mitigate CVE-2025-3573 by ensuring error messages are properly escaped.
utils/check_translation.php (2)
213-213: Added support for new tr_no_op translation functionThe code correctly adds the newly introduced
tr_no_opfunction to the list of translation functions that the script checks for when analyzing PHP files. This ensures that strings wrapped withtr_no_op()will be properly extracted for inclusion in translation files.
215-215: Updated argument handling for tr_no_op functionThe code appropriately includes the
tr_no_opfunction in the same argument handling logic astr,tr_js, andtr_raw, which ensures that the translation extraction correctly processes strings marked with this new function.application/models/Kalkun_model.php (1)
692-692: Enhanced security with error message escaping and translationThe change correctly wraps the phone number validation error message with the
tr()function, which provides two important benefits:
- It translates the error message based on the user's language settings
- It applies HTML entity encoding via
htmlentities()to prevent potential XSS attacksThis change is part of the security improvements to mitigate CVE-2025-3573 by ensuring error messages are properly escaped.
application/controllers/Messages.php (1)
1602-1602: Good security enhancement for error message translation.Wrapping the phone number validation error message with the
tr()function ensures the message is properly translated and HTML-escaped before being displayed to the user, which helps prevent potential XSS vulnerabilities.application/controllers/Phonebook.php (1)
412-412: Good security enhancement for error message translation.Wrapping the phone number validation error message with the
tr()function ensures proper translation and HTML encoding, maintaining consistency with similar changes in other controllers. This helps prevent potential XSS attacks through error messages.application/helpers/kalkun_helper.php (1)
458-458: Improved translation handling for validation messages.Using
tr_no_op()instead of direct string assignment marks the error message for translation extraction without altering it at runtime. This ensures the message will be properly included in translation files while maintaining the original functionality.application/controllers/Kalkun.php (2)
377-380: Good internationalization enhancement for validation messages.Wrapping the error message with
tr_raw()ensures the validation result is properly translated before being returned to the client, maintaining consistency with the internationalization approach used throughout the application.
405-405: Improved error message translation in multiple phone validation.The error message is now properly translated using
tr_raw()before being returned to the client. This enhances the user experience by providing localized validation errors while maintaining the required format with the problematic phone number.application/views/main/phonebook/contact/add_contact.php (4)
71-98: Security enhancement properly implemented withescapeHtml: true.The changes properly implement the recommended security fix for CVE-2025-3573 by adding the
escapeHtml: trueoption to the jQuery validation configuration. This prevents potential XSS attacks by escaping HTML in validation error messages. The implementation also includes proper validation rules and translated error messages.
79-88: Remote validation implementation looks good.The remote validation for phone numbers is properly implemented with appropriate URL and data handling. The GET request to the validation endpoint will help ensure valid phone numbers while providing translated error messages.
90-97: Properly localized validation messages.Good job using the
tr_js()helper function to ensure error messages are properly translated and JSON-encoded for JavaScript. This maintains a consistent localization approach throughout the application.
71-72:✅ Verification successful
Verify jQuery validation plugin version compatibility.
Ensure that your jQuery validation plugin version supports the
escapeHtmloption. This option was introduced in a specific version to address the security vulnerability mentioned in CVE-2025-3573.
🏁 Script executed:
#!/bin/bash # Check the jQuery validation plugin version in use grep -r "jquery.validate" --include="*.php" --include="*.js" .Length of output: 1788
🏁 Script executed:
#!/bin/bash # Locate all jquery.validate.min.js files and print their header comments to determine the version find . -type f -name "jquery.validate.min.js" -exec sed -n '1,10p' {} \;Length of output: 25381
Confirmed compatibility with
escapeHtmlin jQuery Validation Plugin v1.21.0We’re loading
jquery.validate.min.jsv1.21.0 (dated 7/17/2024), and this build already checksthis.settings.escapeHtmlwhen rendering error labels. No upgrade or additional changes are needed.
|
@kingster, can you please review? I'd like to release a new version with this fix soon to fix the package in Debian (hard freeze starts 15/05). |
Initially reported at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1104134
Hence update jquery-validation & use
escapeHtml: true,(as recommended at jquery-validation/jquery-validation#2462).Summary by CodeRabbit
Bug Fixes
New Features
Style
Documentation
Chores