From 3884549890bacc914710ea05d8a165132126f8ae Mon Sep 17 00:00:00 2001 From: edwh Date: Thu, 30 Jul 2026 11:01:02 +0100 Subject: [PATCH 01/14] security: sanitise rich-text descriptions on write Group and event descriptions (`free_text`) are Quill-authored HTML that we render unescaped - the event one on the public /party/view/{id} page - and nothing sanitised them. A host could store a script payload that fired for every visitor, including admins, and there is no CSP to blunt it. Escaping is not an option for a rich-text field, so sanitise with HTMLPurifier (stevebauman/purify, the package the iFixit fork used) in the model mutator rather than in the controllers, so every write path - v2 API, web forms, imports, seeders - is covered by one rule. Network and tag descriptions get the same treatment because NetworkPage.vue renders them with v-html. free_text has two independent render paths: the Blade modals, and GroupDescription/EventDescription -> ReadMore.vue's v-html fed from the API. Sanitising on write covers both; fixing only the Blade side would not have. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Hd9V5FKGKbghx4ztS3rPtF --- app/Group.php | 10 ++++ app/GroupTags.php | 8 +++ app/Network.php | 9 ++++ app/Party.php | 9 ++++ composer.json | 1 + composer.lock | 129 +++++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 165 insertions(+), 1 deletion(-) diff --git a/app/Group.php b/app/Group.php index d61feb3f5c..1a2dbebb32 100644 --- a/app/Group.php +++ b/app/Group.php @@ -538,6 +538,16 @@ public function setDistanceAttribute($val) $this->distance = $val; } + /** + * The group description is Quill-authored HTML which we render unescaped, so it has to + * be sanitised. Doing it in the mutator rather than in the controllers means every + * write path - v2 API, web forms, imports, seeders - is covered by one rule. + */ + public function setFreeTextAttribute($val) + { + $this->attributes['free_text'] = is_null($val) ? null : \Stevebauman\Purify\Facades\Purify::clean($val); + } + public function createDiscourseGroup() { // Get the host who created the group. $success = false; diff --git a/app/GroupTags.php b/app/GroupTags.php index 0fb7af1294..add7c175db 100644 --- a/app/GroupTags.php +++ b/app/GroupTags.php @@ -23,6 +23,14 @@ class GroupTags extends Model */ protected $fillable = ['tag_name', 'description', 'network_id']; + /** + * Tag descriptions are rendered with v-html in NetworkPage.vue, so sanitise on write. + */ + public function setDescriptionAttribute($val) + { + $this->attributes['description'] = is_null($val) ? null : \Stevebauman\Purify\Facades\Purify::clean($val); + } + /** * The attributes that should be hidden for arrays. * diff --git a/app/Network.php b/app/Network.php index 231e3c4d2f..eb4069c18a 100644 --- a/app/Network.php +++ b/app/Network.php @@ -12,6 +12,15 @@ class Network extends Model { use HasFactory; + /** + * The network description is rendered with v-html in NetworkPage.vue, so sanitise it + * on write for the same reason as Group::setFreeTextAttribute(). + */ + public function setDescriptionAttribute($val) + { + $this->attributes['description'] = is_null($val) ? null : \Stevebauman\Purify\Facades\Purify::clean($val); + } + /** * Get tags belonging to this network. */ diff --git a/app/Party.php b/app/Party.php index cfc6a8a4fd..ac9ab31e9a 100644 --- a/app/Party.php +++ b/app/Party.php @@ -774,6 +774,15 @@ public function setEventEndUtcAttribute($val) { $this->attributes['event_end_utc'] = $dt->toDateTimeString(); } + /** + * The event description is Quill-authored HTML which we render unescaped - on a public + * page - so it has to be sanitised. Done in the mutator so every write path is + * covered by one rule. + */ + public function setFreeTextAttribute($val) { + $this->attributes['free_text'] = is_null($val) ? null : \Stevebauman\Purify\Facades\Purify::clean($val); + } + // Mutators for previous event_date/start/end fields. These are now superceded by the UTC fields and therefore // should never be set directly. Throw exceptions to ensure that they are not. public function setEventDateAttribute($val) { diff --git a/composer.json b/composer.json index 08ef157820..dac1f7f737 100644 --- a/composer.json +++ b/composer.json @@ -41,6 +41,7 @@ "spatie/calendar-links": "^1.6", "spatie/laravel-validation-rules": "^3.4", "spinen/laravel-discourse-sso": "^2.8", + "stevebauman/purify": "^6.0", "symfony/http-client": "^6.2", "symfony/http-foundation": "^6.2", "symfony/mailgun-mailer": "^6.2", diff --git a/composer.lock b/composer.lock index 1903d38c14..d078b4c20f 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "bc96c0e7ddaad3d938052527512d5360", + "content-hash": "b47f3e23a394eb8308341eb299c09e40", "packages": [ { "name": "addwiki/mediawiki-api", @@ -1615,6 +1615,67 @@ ], "time": "2023-06-01T07:04:22+00:00" }, + { + "name": "ezyang/htmlpurifier", + "version": "v4.19.0", + "source": { + "type": "git", + "url": "https://github.com/ezyang/htmlpurifier.git", + "reference": "b287d2a16aceffbf6e0295559b39662612b77fcf" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/ezyang/htmlpurifier/zipball/b287d2a16aceffbf6e0295559b39662612b77fcf", + "reference": "b287d2a16aceffbf6e0295559b39662612b77fcf", + "shasum": "" + }, + "require": { + "php": "~5.6.0 || ~7.0.0 || ~7.1.0 || ~7.2.0 || ~7.3.0 || ~7.4.0 || ~8.0.0 || ~8.1.0 || ~8.2.0 || ~8.3.0 || ~8.4.0 || ~8.5.0" + }, + "require-dev": { + "cerdic/css-tidy": "^1.7 || ^2.0", + "simpletest/simpletest": "dev-master" + }, + "suggest": { + "cerdic/css-tidy": "If you want to use the filter 'Filter.ExtractStyleBlocks'.", + "ext-bcmath": "Used for unit conversion and imagecrash protection", + "ext-iconv": "Converts text to and from non-UTF-8 encodings", + "ext-tidy": "Used for pretty-printing HTML" + }, + "type": "library", + "autoload": { + "files": [ + "library/HTMLPurifier.composer.php" + ], + "psr-0": { + "HTMLPurifier": "library/" + }, + "exclude-from-classmap": [ + "/library/HTMLPurifier/Language/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "LGPL-2.1-or-later" + ], + "authors": [ + { + "name": "Edward Z. Yang", + "email": "admin@htmlpurifier.org", + "homepage": "http://ezyang.com" + } + ], + "description": "Standards compliant HTML filter written in PHP", + "homepage": "http://htmlpurifier.org/", + "keywords": [ + "html" + ], + "support": { + "issues": "https://github.com/ezyang/htmlpurifier/issues", + "source": "https://github.com/ezyang/htmlpurifier/tree/v4.19.0" + }, + "time": "2025-10-17T16:34:55+00:00" + }, { "name": "filp/whoops", "version": "2.18.4", @@ -6999,6 +7060,72 @@ }, "time": "2024-04-14T21:40:02+00:00" }, + { + "name": "stevebauman/purify", + "version": "v6.3.2", + "source": { + "type": "git", + "url": "https://github.com/stevebauman/purify.git", + "reference": "deba4aa55a45a7593c369b52d481c87b545a5bf8" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/stevebauman/purify/zipball/deba4aa55a45a7593c369b52d481c87b545a5bf8", + "reference": "deba4aa55a45a7593c369b52d481c87b545a5bf8", + "shasum": "" + }, + "require": { + "ezyang/htmlpurifier": "^4.17", + "illuminate/contracts": "^7.0|^8.0|^9.0|^10.0|^11.0|^12.0|^13.0", + "illuminate/support": "^7.0|^8.0|^9.0|^10.0|^11.0|^12.0|^13.0", + "php": ">=7.4" + }, + "require-dev": { + "orchestra/testbench": "^5.0|^6.0|^7.0|^8.0|^9.0|^10.0|^11.0", + "phpunit/phpunit": "^8.0|^9.0|^10.0|^11.5.3|^12.5.12" + }, + "type": "library", + "extra": { + "laravel": { + "aliases": { + "Purify": "Stevebauman\\Purify\\Facades\\Purify" + }, + "providers": [ + "Stevebauman\\Purify\\PurifyServiceProvider" + ] + } + }, + "autoload": { + "psr-4": { + "Stevebauman\\Purify\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Steve Bauman", + "email": "steven_bauman@outlook.com" + } + ], + "description": "An HTML Purifier / Sanitizer for Laravel", + "keywords": [ + "Purifier", + "clean", + "cleaner", + "html", + "laravel", + "purification", + "purify" + ], + "support": { + "issues": "https://github.com/stevebauman/purify/issues", + "source": "https://github.com/stevebauman/purify/tree/v6.3.2" + }, + "time": "2026-03-18T16:42:42+00:00" + }, { "name": "swagger-api/swagger-ui", "version": "v5.28.0", From f3ebacf962d8cbf6246720bcd66cb84bb74684ed Mon Sep 17 00:00:00 2001 From: edwh Date: Thu, 30 Jul 2026 11:01:02 +0100 Subject: [PATCH 02/14] security: escape user data reaching unescaped output sinks Blade's @lang() compiles to a bare echo and the translator does not escape its :placeholder replacements, while many of the strings wrap those placeholders in markup. Flash messages have the same shape and are rendered with {!! !!}. The audit-log accordion is the residual half of the previously reported audit-log XSS: line 23 (the modified-values cell) was escaped, but line 8 renders `@lang(...metadata, $audit->getMetadata())`, and two of those values are attacker-controlled - user_name is a display name, and audit_url is the request URL including its query string, since develop resolves it with the stock UrlResolver. That fires for any host, coordinator or admin who reviews a group's history. Escape the values rather than the strings, so the intended markup survives. Also covers profile.no_bio (any user's own display name), the event/group description modal headers, share-stats and invite modals, the navbar network name, and the now_following / delete_succeeded / soft_deleted / you_have_joined flash messages. The same unescaped-placeholder bug exists a second time in the client: the Vue translator in lang-utils.js interpolates with a plain .replace(). Add an escapeHtml helper to the lang mixin and use it in GroupPage.vue, the one place a group name reaches a v-html binding. Dynamic translation *keys* taken from DB rows (skill and role names) echo the key verbatim when there is no translation, so those move to {{ __() }} too - one of them renders on the public registration page. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Hd9V5FKGKbghx4ztS3rPtF --- app/Http/Controllers/GroupController.php | 14 +++++++++++--- app/Http/Middleware/AcceptUserInvites.php | 8 ++++---- resources/js/components/GroupPage.vue | 4 +++- resources/js/mixins/lang.js | 15 +++++++++++++++ resources/views/auth/register-new.blade.php | 2 +- resources/views/events/edit.blade.php | 2 +- resources/views/events/view.blade.php | 2 +- .../includes/modals/event-description.blade.php | 2 +- .../includes/modals/event-invite-to.blade.php | 2 +- .../includes/modals/event-share-stats.blade.php | 2 +- .../includes/modals/group-description.blade.php | 2 +- .../includes/modals/group-share-stats.blade.php | 2 +- resources/views/layouts/navbar.blade.php | 2 +- .../networks/partials/add-group-modal.blade.php | 2 +- resources/views/partials/log-accordion.blade.php | 10 ++++++++-- resources/views/user/all.blade.php | 4 ++-- resources/views/user/profile-new.blade.php | 2 +- resources/views/user/profile/profile.blade.php | 6 +++--- 18 files changed, 57 insertions(+), 26 deletions(-) diff --git a/app/Http/Controllers/GroupController.php b/app/Http/Controllers/GroupController.php index 7ae9c17d39..deb7f86eb8 100644 --- a/app/Http/Controllers/GroupController.php +++ b/app/Http/Controllers/GroupController.php @@ -384,7 +384,7 @@ public function postSendInvite(Request $request): RedirectResponse // Don't log to Sentry - legitimate user error. return redirect()->back()->with('warning', __('groups.invite_success_apart_from', [ - 'emails' => rtrim(implode(', ', $not_sent)) + 'emails' => e(rtrim(implode(', ', $not_sent))) ])); } @@ -473,7 +473,7 @@ public function delete($id): RedirectResponse return redirect('/user/forbidden'); } else { return redirect('/group')->with('success', __('groups.delete_succeeded', [ - 'name' => $name, + 'name' => e($name), ])); } } else { @@ -602,7 +602,7 @@ public function getJoinGroup($group_id): RedirectResponse return redirect() ->back() ->with('success', __('groups.now_following', [ - 'name' => $group->name, + 'name' => e($group->name), 'link' => url('/group/view/'.$group->idgroups), ])); } catch (\Exception $e) { @@ -615,6 +615,14 @@ public function getJoinGroup($group_id): RedirectResponse public function imageUpload(Request $request, $id) { + // Same rule as ajaxDeleteImage below - uploading replaces the group's existing + // image, so it needs the same authority as deleting it. + $user = Auth::user(); + + if (! Fixometer::hasRole($user, 'Administrator') && ! Fixometer::userHasEditGroupPermission($id, $user->id)) { + abort(403); + } + try { if (isset($_FILES) && ! empty($_FILES)) { $existing_image = Fixometer::hasImage($id, 'groups', true); diff --git a/app/Http/Middleware/AcceptUserInvites.php b/app/Http/Middleware/AcceptUserInvites.php index 25b1d42cf7..4658374340 100644 --- a/app/Http/Middleware/AcceptUserInvites.php +++ b/app/Http/Middleware/AcceptUserInvites.php @@ -45,13 +45,13 @@ public function handle(Request $request, Closure $next): Response $acceptance->delete(); $request->session()->push('invites-feedback', __('groups.you_have_joined', [ 'url' => url("/group/view/{$group->idgroups}"), - 'name' => $group->name + 'name' => e($group->name) ])); // Else that must mean the User is already part of the Group. // We can then delete the Invite and create a new session } else { - $request->session()->push('invites-feedback', 'You are already a member of idgroups}").'">'.e($group->name).''); } } $request->session()->forget('groups'); @@ -78,13 +78,13 @@ public function handle(Request $request, Closure $next): Response $acceptance->delete(); $request->session()->push('invites-feedback', __('events.you_have_joined', [ 'url' => url("/party/view/{$event->idevents}"), - 'name' => $event->venue + 'name' => e($event->venue) ])); // Else that must mean the User is already part of the Event. // We can then delete the Invite and create a new session } else { - $request->session()->push('invites-feedback', 'You are already a member of idevents}").'">'.e($event->venue).''); } } $request->session()->forget('events'); diff --git a/resources/js/components/GroupPage.vue b/resources/js/components/GroupPage.vue index 49b2a14360..51f07f58bc 100644 --- a/resources/js/components/GroupPage.vue +++ b/resources/js/components/GroupPage.vue @@ -159,8 +159,10 @@ export default { return this.$store.getters['groups/get'](this.idgroups) }, translatedHaveLeft() { + // The string wraps :name in an and this is rendered with v-html, so the group + // name has to be escaped here. return this.__('groups.now_unfollowed', { - name: this.group.name, + name: this.escapeHtml(this.group.name), link: '/group/view/' + this.group.id }) } diff --git a/resources/js/mixins/lang.js b/resources/js/mixins/lang.js index 9128e8e681..38c4af0cbb 100644 --- a/resources/js/mixins/lang.js +++ b/resources/js/mixins/lang.js @@ -33,6 +33,21 @@ export default { Sentry.captureMessage("Missing translation " + key) return key } + }, + // Translation strings can contain markup, and the translator interpolates + // :placeholder values into it without escaping. Any value that comes from the + // database has to be escaped by the caller before it reaches a v-html binding. + escapeHtml(value) { + if (value === null || value === undefined) { + return '' + } + + return String(value) + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, ''') } } } diff --git a/resources/views/auth/register-new.blade.php b/resources/views/auth/register-new.blade.php index 02003a279a..0b4f4ce71f 100644 --- a/resources/views/auth/register-new.blade.php +++ b/resources/views/auth/register-new.blade.php @@ -43,7 +43,7 @@ @foreach ($skills[$key] as $skill)
id, old('skills')) ) checked @endif type="checkbox" name="skills[]" id="skill-{{ $skill->id }}" class="styled-checkbox" value="{{ $skill->id }}"> - +
@endforeach diff --git a/resources/views/events/edit.blade.php b/resources/views/events/edit.blade.php index 4dc4530a13..ade72d1e02 100644 --- a/resources/views/events/edit.blade.php +++ b/resources/views/events/edit.blade.php @@ -17,7 +17,7 @@

- @lang('events.editing', ['event' => ''. $formdata['venue'] .'']) + @lang('events.editing', ['event' => ''. e($formdata['venue']) .''])

diff --git a/resources/views/events/view.blade.php b/resources/views/events/view.blade.php index fb0aff8ae3..8908ae38d5 100644 --- a/resources/views/events/view.blade.php +++ b/resources/views/events/view.blade.php @@ -42,7 +42,7 @@ @if (\Session::has('prompt-follow-group'))
-
@lang('events.follow_hosting_group', ['group' => $event->theGroup->name])
+
@lang('events.follow_hosting_group', ['group' => e($event->theGroup->name)])
@lang('groups.join_group_button')
diff --git a/resources/views/includes/modals/event-description.blade.php b/resources/views/includes/modals/event-description.blade.php index 7eece6a26d..b3d74e1684 100644 --- a/resources/views/includes/modals/event-description.blade.php +++ b/resources/views/includes/modals/event-description.blade.php @@ -6,7 +6,7 @@
-
@lang('events.about_event_name_header', ['event' => $formdata->venue])
+
@lang('events.about_event_name_header', ['event' => e($formdata->venue)])
@include('partials.cross')
diff --git a/resources/views/includes/modals/event-invite-to.blade.php b/resources/views/includes/modals/event-invite-to.blade.php index 29fd70e9ce..df1839d10d 100644 --- a/resources/views/includes/modals/event-invite-to.blade.php +++ b/resources/views/includes/modals/event-invite-to.blade.php @@ -68,7 +68,7 @@

diff --git a/resources/views/includes/modals/event-share-stats.blade.php b/resources/views/includes/modals/event-share-stats.blade.php index f6c82359be..0a5035653e 100644 --- a/resources/views/includes/modals/event-share-stats.blade.php +++ b/resources/views/includes/modals/event-share-stats.blade.php @@ -14,7 +14,7 @@
-

@lang('events.share_stats_message', ['date' => $event->getFormattedLocalStart(), 'event_name' => $formdata->venue, 'number_devices' => $stats['fixed_devices']])

+

@lang('events.share_stats_message', ['date' => $event->getFormattedLocalStart(), 'event_name' => e($formdata->venue), 'number_devices' => $stats['fixed_devices']])