The content of the "emailregexpdesc" parameter is not escaped when displayed to the user

RESOLVED FIXED in Bugzilla 4.0

Status

()

--
minor
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: LpSolit, Assigned: LpSolit)

Tracking

Bugzilla 4.0
Bug Flags:
approval +
approval4.2 +
approval4.0 +

Details

(Whiteboard: [infrasec:xss][ws:low])

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
Created attachment 585318 [details] [diff] [review]
patch, v1

When a user enters an illegal email address when creating or updating his user account, an error message is displayed explaining why his new email address is rejected. The explanation comes from the "emailregexpdesc" parameter directly. The problem is that the content of this parameter is not HTML-escaped, which permits to inject code into the page, or totally break the layout of the page.

This is not a major issue, because this parameter is controlled by admins, but it's easy to accidently insert < or > into the text and so let the browser interpret them as HTML tags.

My patch uses FILTER html_light instead of FILTER html to still let admins inject some safe HTML entities, such as links or bold text. This is the filter we use for all other admin-controlled descriptions (such as the ones for products and components).
Attachment #585318 - Flags: review?(dkl)
Whiteboard: [infrasec:xss][ws:low]
Any reason why the filter test didn't catch this one?
(Assignee)

Comment 2

7 years ago
(In reply to Reed Loden [:reed] (very busy) from comment #1)
> Any reason why the filter test didn't catch this one?

Param('foo') is always considered safe, which is why 008filter.t doesn't complain. But in a separate bug, we should remove this rule, and force Param('foo') to be always filtered, at least with FILTER none if we really want no filtering.

See also bug 159631 for a related bug about Param('maintainer').
Comment on attachment 585318 [details] [diff] [review]
patch, v1

Review of attachment 585318 [details] [diff] [review]:
-----------------------------------------------------------------

Works good. r=dkl
Attachment #585318 - Flags: review?(dkl) → review+
(Assignee)

Updated

7 years ago
Flags: approval4.2+
Flags: approval4.0+
Flags: approval+
(Assignee)

Comment 4

7 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified template/en/default/global/code-error.html.tmpl
modified template/en/default/global/user-error.html.tmpl
Committed revision 8058.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified template/en/default/global/code-error.html.tmpl
modified template/en/default/global/user-error.html.tmpl
Committed revision 7995.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified template/en/default/global/code-error.html.tmpl
modified template/en/default/global/user-error.html.tmpl
Committed revision 7676.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.