2014-11-27 00:16 EST

View Issue Details Jump to Notes ] Wiki ] Related Changesets ]
IDProjectCategoryView StatusLast Update
0015384mantisbtsecuritypublic2014-09-23 18:05
Reporteratrol 
Assigned Todhx 
PriorityurgentSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
Product Version1.2.12 
Target Version1.2.13Fixed in Version1.2.13 
Summary0015384: CVE-2013-1810 XSS vulnerability on summary page
DescriptionScript is executed when viewing summary page and having a category with scripting code

This has been introduced by a commit to resolve 0014677

I will attach a patch.
Please have a look at it and feel free to enhance and to commit it.
Steps To Reproduce1. Create a category <script>alert ("XSS")</script>
2. View "Summary" page
TagsNo tags attached.
Attached Files
  • patch file icon fix15384.patch (1,858 bytes) 2013-01-18 15:26 - 
    From 353c8741a797f56f1ae7df078b1dd7ceac49713e Mon Sep 17 00:00:00 2001
    From: Roland Becker <roland@atrol.de>
    Date: Fri, 18 Jan 2013 21:24:13 +0100
    Subject: [PATCH] Fix #15384 XSS vulnerability on summary page
    
    ---
     core/summary_api.php |    9 ++++++---
     1 files changed, 6 insertions(+), 3 deletions(-)
    
    diff --git a/core/summary_api.php b/core/summary_api.php
    index abdbbc4..638cabf 100644
    --- a/core/summary_api.php
    +++ b/core/summary_api.php
    @@ -27,7 +27,10 @@
      */
     require_once( $g_absolute_path . 'config_filter_defaults_inc.php' );
     
    -function summary_helper_print_row( $p_label, $p_open, $p_resolved, $p_closed, $p_total ) {
    +function summary_helper_print_row( $p_label, $p_open, $p_resolved, $p_closed, $p_total, $p_sanitize_label = true ) {
    +	if ( $p_sanitize_label ) {
    +		$p_label = string_display_line ( $p_label );
    +	}
     	printf( '<tr %s>', helper_alternate_class() );
     	printf( '<td width="50%%">%s</td>', $p_label );
     	printf( '<td width="12%%" class="right">%s</td>', $p_open );
    @@ -464,7 +467,7 @@ function summary_print_by_developer() {
     				$t_bugs_total = $t_bug_link . '&amp;' . FILTER_PROPERTY_HIDE_STATUS_ID . '=">' . $t_bugs_total . '</a>';
     			}
     
    -			summary_helper_print_row( $t_user, $t_bugs_open, $t_bugs_resolved, $t_bugs_closed, $t_bugs_total );
    +			summary_helper_print_row( $t_user, $t_bugs_open, $t_bugs_resolved, $t_bugs_closed, $t_bugs_total, false );
     
     			$t_bugs_open = 0;
     			$t_bugs_resolved = 0;
    @@ -501,7 +504,7 @@ function summary_print_by_developer() {
     			$t_bugs_total = $t_bug_link . '&amp;' . FILTER_PROPERTY_HIDE_STATUS_ID . '=">' . $t_bugs_total . '</a>';
     		}
     
    -		summary_helper_print_row( $t_user, $t_bugs_open, $t_bugs_resolved, $t_bugs_closed, $t_bugs_total );
    +		summary_helper_print_row( $t_user, $t_bugs_open, $t_bugs_resolved, $t_bugs_closed, $t_bugs_total, false );
     	}
     }
     
    -- 
    1.7.7.1.msysgit.0
    
    
    patch file icon fix15384.patch (1,858 bytes) 2013-01-18 15:26 + 

- Relationships
related to 0015721closedgrangeway Functionality to consider porting to master-2.0.x 
+ Relationships

-  Notes
User avatar

~0034817

atrol (developer)

Reminder sent to: dhx, rombert

Please have a look at the attached patch.
User avatar

~0034819

rombert (developer)

Confirmed, this is another XSS I introduced :|

Overall the fix looks good to me since usernames are not allowed to have special characters therefore there is no XSS risk in allowing special chars in there. Of course, being the guilty party I'll let others judge if this is enough/a proper fix.
User avatar

~0034820

atrol (developer)

I see no problem, even if usernames do contain special chars

function summary_helper_get_developer_label does the sanitizing job if summary_helper_print_row is called with $p_sanitize_label = false

> $t_user = string_display_line( user_get_name( $p_user_id ) );

I don't like my fix that much as this is not a good readable/maintainable code.
At least it does the job to fix the XSS issue without loosing the new functionality that you introduced.

I don't have enough time at the moment to provide a better patch with cleaner code.

It's your decision to commit it as it is or to take some time for a small rewrite.
User avatar

~0034821

dhx (reporter)

Thanks Roland. I've applied a patch that doesn't introduce any new parameters to the summary_helper_print_row function (it'd be messy to do so).

master branch is unaffected by this issue as it already had correct escaping of all data being fed into the summary_helper_print_row function.

Both branches should now be in line w.r.t. escaping input into summary_helper_print_row.
User avatar

~0035362

dhx (reporter)

This was assigned the CVE identifier CVE-2013-1810 on the oss-security mailing list on March 3rd, 2013.
User avatar

~0036085

grangeway (reporter)

Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch
+  Notes

+ Related Changesets

- Issue History
Date Modified Username Field Change
2013-01-18 15:24 atrol New Issue
2013-01-18 15:26 atrol File Added: fix15384.patch
2013-01-18 15:28 atrol Note Added: 0034817
2013-01-18 15:38 rombert Note Added: 0034819
2013-01-18 15:38 rombert Status new => confirmed
2013-01-18 15:38 rombert Description Updated View Revisions
2013-01-18 15:38 rombert Steps to Reproduce Updated View Revisions
2013-01-18 15:56 atrol Note Added: 0034820
2013-01-18 16:41 rombert Priority normal => urgent
2013-01-18 18:05 dhx Note Added: 0034821
2013-01-18 18:05 dhx Assigned To => dhx
2013-01-18 18:05 dhx Severity minor => major
2013-01-18 18:05 dhx Status confirmed => resolved
2013-01-18 18:05 dhx Resolution open => fixed
2013-01-18 18:05 dhx Fixed in Version => 1.2.13
2013-01-18 18:05 dhx Description Updated View Revisions
2013-01-18 18:05 dhx Steps to Reproduce Updated View Revisions
2013-01-18 18:05 dhx View Status private => public
2013-01-18 18:06 dhx Changeset attached => MantisBT master-1.2.x 7df30a9e
2013-01-20 06:30 dregad Relationship added related to 0015388
2013-01-21 04:03 dregad Relationship deleted related to 0015388
2013-01-22 09:47 dregad Status resolved => closed
2013-03-03 00:41 dhx Note Added: 0035362
2013-03-04 11:16 dregad Summary XSS vulnerability on summary page => CVE-2013-1810 XSS vulnerability on summary page
2013-04-05 17:56 grangeway Status closed => acknowledged
2013-04-05 17:56 grangeway Note Added: 0036085
2013-04-05 19:40 grangeway Relationship added related to 0015721
2013-04-06 03:39 dregad Status acknowledged => resolved
2013-04-06 07:20 grangeway Status resolved => acknowledged
2013-04-06 09:22 dregad Tag Attached: 2.0.x check
2013-04-06 09:23 dregad Status acknowledged => closed
2014-09-23 18:05 grangeway Tag Detached: 2.0.x check
+ Issue History