fix(ncp-web): prevent XSS via log output and undefined array key warning#2096
Open
aroldobossoni wants to merge 1 commit intonextcloud:masterfrom
Open
Conversation
This commit addresses two issues in the NextCloudPi web panel:
1. HTML injection via ncp.log content (XSS vulnerability)
- When /var/log/ncp.log contains HTML-like content (e.g., <strftime_format>
from certain backup operations), the unescaped output breaks the HTML parser
- This causes the browser to ignore subsequent <script> tags, preventing
minified.js and ncp.js from loading
- Result: the dashboard never loads and shows infinite "System Info" spinner
- Fix: wrap file_get_contents() with htmlspecialchars() in index.php line 290
2. PHP warning for undefined HTTP_ACCEPT_LANGUAGE
- When requests lack Accept-Language header (e.g., API calls, curl),
PHP emits "Undefined array key" warning
- This warning can corrupt JSON responses from ncp-launcher.php
- Fix: use null coalescing operator (?? '') in both index.php and
ncp-launcher.php
Tested on NextCloudPi running in LXC container on Proxmox.
Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes two issues in the NextCloudPi web panel that can cause the admin dashboard to fail loading:
1. XSS vulnerability / HTML parser breakage via ncp.log content
Problem: When
/var/log/ncp.logcontains HTML-like strings (e.g.,<strftime_format>from backup operations), the unescaped output inindex.phpbreaks the browser's HTML parser. This causes:<script>tags to be ignoredminified.jsandncp.jsnever executeFix: Wrap
file_get_contents('/var/log/ncp.log')withhtmlspecialchars(..., ENT_QUOTES, 'UTF-8')to properly escape HTML entities.2. PHP warning for undefined HTTP_ACCEPT_LANGUAGE array key
Problem: When HTTP requests lack the
Accept-Languageheader (common with API calls, curl, automated scripts), PHP emits an "Undefined array key" warning. Inncp-launcher.php, this warning can corrupt JSON responses.Fix: Use PHP's null coalescing operator (
?? '') to provide a default empty string when the header is missing.Files Changed
ncp-web/index.php(lines 68, 290)ncp-web/ncp-launcher.php(line 24)Testing
Tested on NextCloudPi running in an LXC container on Proxmox. After applying these fixes:
Related Issues
This addresses a real-world issue where users see an infinite loading spinner on the "System Info" section of the NCP admin panel.