Skip to content

Sanitise gl_field_error error message

Scott de Jonge requested to merge 439865-sanitise-field-error into master

What does this MR do and why?

Fixes #439865 (closed). Please read the issue for details. Especially here where it can't be exploited on its own (it requires an XSS vulnerability in the product) so can be fixed in public security-fix-in-public.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Before After
CleanShot_2024-02-16_at_16.30.23_2x CleanShot_2024-02-16_at_16.35.30_2x

How to set up and validate locally

  1. Navigate to form field which uses gl-show-field-errors class e.g. http://gdk.test:3000/-/user_settings/profile
  2. Add XSS exploit to title attribute on <input> e.g.
diff --git a/ee/app/views/user_settings/profiles/_name.html.haml b/ee/app/views/user_settings/profiles/_name.html.haml
index a4163175bf7d..19e2b38a1048 100644
--- a/ee/app/views/user_settings/profiles/_name.html.haml
+++ b/ee/app/views/user_settings/profiles/_name.html.haml
@@ -4,7 +4,7 @@
   %small.form-text.text-gl-muted
     = s_("Profiles|Your name was automatically set based on your %{provider_label} account, so people you know can recognize you") % { provider_label: attribute_provider_label(:name) }
 - elsif can?(current_user, :update_name, user)
-  = form.text_field :name, class: 'gl-form-input form-control', required: true, title: s_("Profiles|Using emoji in names seems fun, but please try to set a status message instead")
+  = form.text_field :name, class: 'gl-form-input form-control', required: true, title: '<script>alert(document.domain)</script>'
   %small.form-text.text-gl-muted
     = s_("Profiles|Enter your name, so people you know can recognize you.")
     = safe_format(s_("Profiles|No \"&lt;\" or \"&gt;\" characters, please."))
  1. View page with <input> element with XSS exploit, alert() should not be displayed with sanitise() change in this MR

Merge request reports