View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0021611 | mantisbt | security | public | 2016-08-15 04:57 | 2016-08-28 01:12 |
Reporter | wdollman | Assigned To | dregad | ||
Priority | normal | Severity | major | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Product Version | 1.2.0 | ||||
Target Version | 1.3.1 | Fixed in Version | 1.3.1 | ||
Summary | 0021611: CVE-2016-6837: XSS vulnerability in view_all_bug_page.php | ||||
Description | The value of the view_type parameter on the view_all_bug_page.php page is not encoded before being displayed on the page. For example, browsing to /view_all_bug_page.php?view_type=">injected-html results in the following HTML: <input type="hidden" name="view_type" value="">injected-html" /> This allows arbitrary JavaScript to be injected into the page, and affects the latest stable release (1.3.0). | ||||
Steps To Reproduce | This installation of Mantis is vulnerable to the issue, and runs with a Content-Security-Policy that allows inline JavaScript. Visiting the following URL in a browser with no cross-site scripting filter (e.g. Mozilla Firefox), or a browser with the cross-site scripting filter disabled, will trigger a JavaScript popup: http://mantisbt.org/bugs/view_all_bug_page.php?view_type="><script>alert(1);</script> A general example is /view_all_bug_page.php?view_type="><script>alert(1);</script> This proof-of-concept is blocked by the default Content-Security-Policy set by Mantis 1.3.0, though as shown above, some sites may have disabled or relaxed the default CSP. | ||||
Additional Information | If you request a CVE for this issue, please credit Will Dollman of Netcraft Ltd. | ||||
Tags | No tags attached. | ||||
Attached Files | 0001-Fix-XSS-in-view_all_bug_page.php.patch (1,293 bytes)
From 421af1ed47a2b9a72c999876e6e17c7f621acb5f Mon Sep 17 00:00:00 2001 From: Damien Regad <dregad@mantisbt.org> Date: Tue, 16 Aug 2016 23:22:40 +0200 Subject: [PATCH] Fix XSS in view_all_bug_page.php The value of the view_type parameter on the view_all_bug_page.php page was not encoded before being displayed. This vulnerability was discovered by Will Dollman of Netcraft Ltd. Fixes #21611 --- core/filter_api.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/filter_api.php b/core/filter_api.php index c8e8eae..9294f64 100644 --- a/core/filter_api.php +++ b/core/filter_api.php @@ -489,9 +489,15 @@ function filter_ensure_valid_filter( array $p_filter_arr ) { # if the version is old, update it $p_filter_arr['_version'] = FILTER_VERSION; } + + # Filter type - ensure it's either 'simple' or 'advanced' (prevent XSS) if( !isset( $p_filter_arr['_view_type'] ) ) { $p_filter_arr['_view_type'] = gpc_get_string( 'view_type', 'simple' ); } + if( $p_filter_arr['_view_type'] != 'advanced' ) { + $p_filter_arr['_view_type'] = 'simple'; + } + if( !isset( $p_filter_arr[FILTER_PROPERTY_ISSUES_PER_PAGE] ) ) { $p_filter_arr[FILTER_PROPERTY_ISSUES_PER_PAGE] = gpc_get_int( FILTER_PROPERTY_ISSUES_PER_PAGE, config_get( 'default_limit_view' ) ); } -- 2.7.4 0001-Fix-XSS-in-view_all_bug_page.php-safecmp.patch (1,294 bytes)
From 421af1ed47a2b9a72c999876e6e17c7f621acb5f Mon Sep 17 00:00:00 2001 From: Damien Regad <dregad@mantisbt.org> Date: Tue, 16 Aug 2016 23:22:40 +0200 Subject: [PATCH] Fix XSS in view_all_bug_page.php The value of the view_type parameter on the view_all_bug_page.php page was not encoded before being displayed. This vulnerability was discovered by Will Dollman of Netcraft Ltd. Fixes #21611 --- core/filter_api.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/filter_api.php b/core/filter_api.php index c8e8eae..9294f64 100644 --- a/core/filter_api.php +++ b/core/filter_api.php @@ -489,9 +489,15 @@ function filter_ensure_valid_filter( array $p_filter_arr ) { # if the version is old, update it $p_filter_arr['_version'] = FILTER_VERSION; } + + # Filter type - ensure it's either 'simple' or 'advanced' (prevent XSS) if( !isset( $p_filter_arr['_view_type'] ) ) { $p_filter_arr['_view_type'] = gpc_get_string( 'view_type', 'simple' ); } + if( $p_filter_arr['_view_type'] !== 'advanced' ) { + $p_filter_arr['_view_type'] = 'simple'; + } + if( !isset( $p_filter_arr[FILTER_PROPERTY_ISSUES_PER_PAGE] ) ) { $p_filter_arr[FILTER_PROPERTY_ISSUES_PER_PAGE] = gpc_get_int( FILTER_PROPERTY_ISSUES_PER_PAGE, config_get( 'default_limit_view' ) ); } -- 2.7.4 | ||||
Changing line 2258 in file core/filter_api.php should fix the issue
|
|
I've included our patch for the issue below. It sanitizes the variable in filter_ensure_valid_filter(), so it's safe for display even if it's echoed elsewhere in a future change: <pre> @@ -492,6 +492,11 @@ function filter_ensure_valid_filter( array $p_filter_arr ) {
|
|
The issue actually exists (at least) since 1.2.0. |
|
I believe the better solution is indeed to ensure that whatever value we store in the filter array is properly sanitized, to guarantee that any future use is safe. Please see proposed patch attached. @wdollman kindly confirm that this indeed fixes the issue; upon receiving your confirmation, I'll request a CVE. Note: my fix improves the performance of the code proposed in 0021611:0053809, as it does not rely on calling the in_array() function. |
|
I've attached an updated patch with a minor change which swaps the != comparison for a !== If $p_filter_arr['_view_type'] were set to 0 (integer), PHP will cast "advanced" to an integer, bypassing the != check and we'd end up with a sanitized $p_filter_arr['_view_type'] value of 0. I don't see any way of triggering this - gpc_get_string() returns a string rather than an integer, and also don't see any way it could be exploited - but I don't like the check returning an unexpected value. If you're ok with this updated patch, I'm happy that it resolves the issue and for you to request a CVE. |
|
Thanks for the feedback Will. I have no objections with your proposed use of the strict comparison operator, even though as you pointed out I don't think it's possible for gpc_string() to return anything other than a string in this context, but it doesn't hurt. Belt and braces ;-) I'll push the modified patch to Github now, and request the CVE later; I will update this issue when I receive the ID. Note: the issue is now public |
|
That is fine, but note that a filter array may not come only from previously stored filter. |
|
You're confusing me. This is actually exactly where I've applied the patch... Did I miss something ? |
|
Maybe i didn't explain right. As you said, the validation must to be done in "filter_ensure_valid_filter". As i read "I believe the better solution is indeed to ensure that whatever value we store in the filter array is properly sanitized": |
|
CVE-2016-6837 assigned. |
|
MantisBT: master-1.3.x 7086c2d8 2016-08-16 13:22 Details Diff |
Fix XSS in view_all_bug_page.php The value of the view_type parameter on the view_all_bug_page.php page was not encoded before being displayed. This vulnerability was discovered by Will Dollman of Netcraft Ltd. Initial patch modified to use strict comparison per Will's suggestion. Fixes 0021611 |
Affected Issues 0021611 |
|
mod - core/filter_api.php | Diff File |