Skip to content

Commit

Permalink
Fix XSS in custom fields management
Browse files Browse the repository at this point in the history
Kacper Szurek (http://security.szurek.pl/) discovered an XSS
vulnerability in Custom fields management pages, caused by unescaped
output of 'return URL' GPC parameter. His report describes two ways to
exploit this issue:

1. using 'accesskey' inside hidden input field (see [1]) reflects XSS to
   the administrator in manage_custom_field_edit_page.php when the
   keyboard shortcut is actioned
2. using 'javascript:' URI scheme executes the code when the user clicks
   the [Proceed] link on manage_custom_field_update.php after updating
   a custom field

This commit fixes both attack vectors:

- properly escape the return URL prior to printing it on the hidden form
  field
- let html_operation_successful() sanitize the URL before displaying
  it, just like html_meta_redirect() does. In this case, if the
  string contains an URI scheme, it will be replaced by 'index.php'

[1] http://blog.portswigger.net/2015/11/xss-in-hidden-input-fields.html

Fixes #20956

This is a backport from master 3f2779b4c6dc8d465fb73c08cfa1d806184d2e79.
  • Loading branch information
dregad committed Jun 6, 2016
1 parent b8d5d85 commit 5068df2
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 6 deletions.
2 changes: 1 addition & 1 deletion account_prefs_update.php
Expand Up @@ -115,6 +115,6 @@
echo lang_get( 'operation_successful' );

echo '<br />';
print_bracket_link( $f_redirect_url, lang_get( 'proceed' ) );
print_bracket_link( string_sanitize_url( $f_redirect_url ), lang_get( 'proceed' ) );
echo '<br /></div>';
html_page_bottom();
2 changes: 1 addition & 1 deletion manage_config_revert.php
Expand Up @@ -65,7 +65,7 @@
<div align="center">
<?php
echo lang_get( 'operation_successful' ).'<br />';
print_bracket_link( $t_redirect_url, lang_get( 'proceed' ) );
print_bracket_link( string_sanitize_url( $t_redirect_url ), lang_get( 'proceed' ) );
?>
</div>

Expand Down
2 changes: 1 addition & 1 deletion manage_custom_field_delete.php
Expand Up @@ -58,7 +58,7 @@
<div align="center">
<?php
echo lang_get( 'operation_successful' ) . '<br />';
print_bracket_link( $f_return, lang_get( 'proceed' ) );
print_bracket_link( string_sanitize_url( $f_return ), lang_get( 'proceed' ) );
?>
</div>

Expand Down
2 changes: 1 addition & 1 deletion manage_custom_field_update.php
Expand Up @@ -64,7 +64,7 @@

echo lang_get( 'operation_successful' ) . '<br />';

print_bracket_link( $f_return, lang_get( 'proceed' ) );
print_bracket_link( string_sanitize_url( $f_return ), lang_get( 'proceed' ) );

echo '</div>';

Expand Down
2 changes: 1 addition & 1 deletion print_all_bug_options_update.php
Expand Up @@ -79,6 +79,6 @@
}

echo '<br />';
print_bracket_link( $f_redirect_url, lang_get( 'proceed' ) );
print_bracket_link( string_sanitize_url( $f_redirect_url ), lang_get( 'proceed' ) );
echo '<br /></div>';
html_page_bottom();
2 changes: 1 addition & 1 deletion set_project.php
Expand Up @@ -109,7 +109,7 @@
<?php
echo lang_get( 'operation_successful' ).'<br />';

print_bracket_link( $t_redirect_url, lang_get( 'proceed' ) );
print_bracket_link( string_sanitize_url( $t_redirect_url ), lang_get( 'proceed' ) );
?>
</div>

Expand Down

0 comments on commit 5068df2

Please sign in to comment.