XSS vulnerability when using the display as links option [#2063107]

I am using the latest version of Better Exposed Filters (7.x-3.0-beta3) with views and believe I have found a XSS vulnerablity. If you use the "Links" display option in views, the hidden tag contents are not properly escaped and you can get javascipt code to execute.

For instance, if you have a term named field_test_tid and set up Better Exposed Filters on a view on the site test.com, then you can use a url such as

http://www.test.com/page_with_view/filtered?field_test_tid=3"style%3d"behavior%3aurl(%23default%23time2)"onbegin%3d"alert('XSS')

to execute javascript.

I think the problem resides in the file better_exposed_filters.theme on lines 441 and 445. The module runs the function filter_xss, however as this code is echoed inside an input tag, the string crafted above

3"style%3d"behavior%3aurl(%23default%23time2)"onbegin%3d"alert('XSS')

passes through and executes javascript on the page.

I recreated this bug using IE6 (6.0.2900.5512) on WinXp(SP3).

Comment	File	Size	Author
#3	bef_xss-2063107.patch	1.03 KB	alexfarr

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

mikeker CreditAttribution: mikeker commented 16 October 2013 at 05:22

Status:

Active

» Closed (cannot reproduce)

I am unable to reproduce this bug. It seems very similar to what was fixed in this bug. Please make sure you're using the latest code.

If you are and can still repo this bug, please attach an export of the view in question.

Finally, if you feel you have a real security issue, please follow the guidelines for reporting Drupal security flaws.

Thanks.

Comment #2

greggles

he/him

English

Denver, Colorado, USA

CreditAttribution: greggles commented 16 October 2013 at 13:21

@mikeker this was reported following the proper process but was then redirected here as the BEF module doesn't have a stable release and so security bugs can be fixed in public. It would be great if you could revert the patch you mention and see if you can replicate the behavior @darmordls found and then apply the patch and see it go away.

Comment #3

alexfarr CreditAttribution: alexfarr commented 21 January 2014 at 16:49

Issue summary:

View changes

File	Size
bef_xss-2063107.patch	1.03 KB

I have had the same problem, the linked patch does not fix all of the xss issues. This patch will fix the issues on the hidden fields when display results as links.

Comment #4

mikeker CreditAttribution: mikeker commented 21 January 2014 at 17:15

@alexfarr: Thanks for the patch, but as mentioned in #1, this has been fixed in the latest -dev release (code change is here). Your patch looks like it's against beta 3.

Can you please retest your setup using the -dev release. If the vulnerability is still there in -dev, then please reopen this issue. (To be honest, I only ended up on this issue by accident because it's still marked as Closed).

Thanks.

Comment #5

bfr CreditAttribution: bfr commented 15 September 2014 at 11:10

Version:	7.x-3.0-beta3	» 7.x-3.x-dev
Status:	Closed (cannot reproduce)	» Active

The problem is still there. While the patch filters some XSS-attacks, it does not touch javascript functions.

Try this:
www.example.com/bef_xss_test?tid=503"%20onerror="alert(503);

Output:

<input type="hidden" name="tid" value="503" onerror="alert(503);" /></div>

I think the problem lies in the filter_xss() core function. Not sure how to fix it.

Comment #6

greggles

he/him

English

Denver, Colorado, USA

CreditAttribution: greggles commented 15 September 2014 at 14:48

filter_xss is not meant to be used like this. filter_xss is intended to run on whole hunks of well-formed html (or at least relatively well formed).

Core doesn't really provide a great function that is perfectly suited to escaping html attributes, but I believe check_plain will at least mitigate the xss issue.

I described this in an article https://docs.acquia.com/articles/using-filter-functions-intended-filterx...

Comment #7

3 November 2014 at 09:02

mikeker committed 0a4406c on 7.x-3.x

Issue #2063107 by mikeker | darmordls: Fixed XSS vulnerability when...

Comment #8

mikeker CreditAttribution: mikeker commented 3 November 2014 at 09:08

I believe the reason to use filter_xss instead of check_plain had to do with filter options that had HTML. But the security issue outweighs pretty displays so check_plain it is.

@greggles: Thanks for the link describing the limitation of filter_xss. I'm curious, if I were to put the HTML attributes into a random tag (say, a div) and then compare the filter_xss'ed version vs the original one, that might highlight when the attribute has something suspicious in it.

Apologies for the delay committing this -- I was blissfully off-line on vacation for the last several weeks! :P