fix: escape release notes from external version server (#7960) - #7968
Open
wakqasahmed wants to merge 1 commit into
Open
fix: escape release notes from external version server (#7960)#7968wakqasahmed wants to merge 1 commit into
wakqasahmed wants to merge 1 commit into
Conversation
…cahq#7960) The self-hosted "what's new" version-check modal rendered Instance::latest_release_notes with Blade's unescaped {!! !!} syntax. That field is populated verbatim from the JSON response of an external server (version.monicahq.com) by the monica:ping scheduled command. A compromised or DNS-hijacked version server could inject arbitrary HTML/JS that executes in every authenticated user's session across all self-hosted instances with version checking enabled. Switched to Blade's escaped {{ }} syntax so the notes are always rendered as text.
|
|
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.
Fixes #7960
Vulnerability
app/Console/Commands/PingVersionServer.php:88stores$json['notes']— fetched from the externalversion.monicahq.comserver by the scheduledmonica:pingcommand — verbatim intoInstance::latest_release_notes, with no sanitization.resources/views/partials/check.blade.php:23then rendered that field using Blade's unescaped output syntax:{!! $instance->latest_release_notes !!}. This partial is included fromresources/views/partials/footer.blade.php, which loads on every authenticated page.If an attacker can influence the response from
version.monicahq.com(DNS hijack, BGP manipulation, or direct compromise of that server), arbitrary HTML/JavaScript gets stored in the database and executes in the browser of every authenticated user, on every self-hosted Monica instance with version checking enabled (the default), whenever they open the "What's new" modal. No individual Monica instance needs to be compromised.Fix
Changed
check.blade.php:23from{!! $instance->latest_release_notes !!}to{{ $instance->latest_release_notes }}, per the issue's own recommended fix — a one-character change that makes Blade HTML-escape the content instead of rendering it raw. This is the precise vector cited in the issue; I did not change the ingestion/fetch code, since escaping at render time closes the vulnerability regardless of what the external server returns.Tests
Added
tests/Unit/Views/CheckVersionBladeTest.php::it_escapes_release_notes_fetched_from_the_version_server, which renderspartials.checkwith anInstancewhoselatest_release_notescontains a<script>payload and asserts the rendered HTML contains the HTML-entity-escaped form, not a live<script>tag.Disclosure: PHP is not available in the environment these changes were authored in, so this test could not be executed locally. It was written to match this repo's existing Pest/PHPUnit test conventions (see
tests/Commands/Scheduling/PingVersionServerTest.phpfor the sameInstancefactory usage) and should be run in CI/a working dev environment before merge.