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).

Files: 
CommentFileSizeAuthor
#3 bef_xss-2063107.patch1.03 KBalexfarr

Comments

mikeker’s picture

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.

greggles’s picture

@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.

alexfarr’s picture

Issue summary:View changes
StatusFileSize
new1.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.

mikeker’s picture

@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.

bfr’s picture

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.

greggles’s picture

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...

  • mikeker committed 0a4406c on 7.x-3.x
    Issue #2063107 by mikeker | darmordls: Fixed XSS vulnerability when...
mikeker’s picture

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

mikeker’s picture

Status:Active» Fixed

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.