Skip to content

Commit 5b93161

Browse files
mantisdavidhicks
authored andcommitted
Rework the bug action group api such that we can easily convert this to an object in the future, and to validate calls to require once.
This leads to a security issue identified by IBM's Appscan program, whereby calls to require_once are not validated. Depending on webserver configuration, this is a file inclusion vulnerability. There will be a follow up commit to config api - probably: - if( $g_project_override != null ) { + if( $g_project_override != null && $p_project == null ) { At the moment, the action group API calls config_get with a project parameter to use. This is ignored, due to project_override being set - so we either need to: a) change project override within the command list function b) modifify config api to only use the project override *if* it is attempting to look up information on the default project. Backported from master-1.2.x branch. Note that this commit relies upon commit 6dc3510 from the master branch (that hadn't been backported to 1.2.x). Conflicts: bug_actiongroup_ext.php bug_actiongroup_ext_page.php bug_actiongroup_page.php core/bug_group_action_api.php Signed-off-by: David Hicks <d@hx.id.au>
1 parent 965b00a commit 5b93161

4 files changed

+26
-28
lines changed

Diff for: bug_actiongroup_ext.php

+1-2
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,11 @@
3535
$f_action = gpc_get_string( 'action' );
3636
$f_bug_arr = gpc_get_int_array( 'bug_arr', array() );
3737

38-
$t_action_include_file = 'bug_actiongroup_' . $f_action . '_inc.php';
3938
$t_form_name = 'bug_actiongroup_' . $f_action;
4039

4140
form_security_validate( $t_form_name );
4241

43-
require_once( dirname( __FILE__ ) . DIRECTORY_SEPARATOR . $t_action_include_file );
42+
bug_group_action_init( $f_action );
4443

4544
# group bugs by project
4645
$t_projects_bugs = array();

Diff for: bug_actiongroup_ext_page.php

+3-22
Original file line numberDiff line numberDiff line change
@@ -20,34 +20,15 @@
2020
* @copyright Copyright (C) 2002 - 2011 MantisBT Team - mantisbt-dev@lists.sourceforge.net
2121
* @link http://www.mantisbt.org
2222
*/
23-
/**
24-
* MantisBT Core API's
25-
*/
26-
require_once( 'core.php' );
2723

24+
require_once( 'core.php' );
2825
require_once( 'bug_group_action_api.php' );
2926

30-
auth_ensure_user_authenticated();
31-
32-
$f_action = gpc_get_string( 'action' );
33-
$f_bug_arr = gpc_get_int_array( 'bug_arr', array() );
34-
35-
# redirect to view issues if nothing is selected
36-
if ( is_blank( $f_action ) || ( 0 == count( $f_bug_arr ) ) ) {
37-
print_header_redirect( 'view_all_bug_page.php' );
38-
}
39-
40-
# redirect to view issues page if action doesn't have ext_* prefix.
41-
# This should only occur if this page is called directly.
42-
$t_external_action_prefix = 'EXT_';
43-
if ( strpos( $f_action, $t_external_action_prefix ) !== 0 ) {
44-
print_header_redirect( 'view_all_bug_page.php' );
45-
}
46-
4727
$t_external_action = utf8_strtolower( utf8_substr( $f_action, utf8_strlen( $t_external_action_prefix ) ) );
48-
$t_form_fields_page = 'bug_actiongroup_' . $t_external_action . '_inc.php';
4928
$t_form_name = 'bug_actiongroup_' . $t_external_action;
5029

30+
bug_group_action_init( $t_external_action );
31+
5132
bug_group_action_print_top();
5233
?>
5334

Diff for: bug_actiongroup_page.php

+3
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
# run through the issues to see if they are all from one project
4343
$t_project_id = ALL_PROJECTS;
4444
$t_multiple_projects = false;
45+
$t_projects = array();
4546

4647
bug_cache_array_rows( $f_bug_arr );
4748

@@ -52,11 +53,13 @@
5253
$t_multiple_projects = true;
5354
} else {
5455
$t_project_id = $t_bug->project_id;
56+
$t_projects[$t_project_id] = $t_project_id;
5557
}
5658
}
5759
}
5860
if ( $t_multiple_projects ) {
5961
$t_project_id = ALL_PROJECTS;
62+
$t_projects[ALL_PROJECTS] = ALL_PROJECTS;
6063
}
6164
# override the project if necessary
6265
if( $t_project_id != helper_get_current_project() ) {

Diff for: core/bug_group_action_api.php

+19-4
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,25 @@
2222
* @subpackage BugGroupActionAPI
2323
*/
2424

25+
/**
26+
* Initialise bug action group api
27+
*/
28+
function bug_group_action_init( $p_action ) {
29+
$t_valid_actions = bug_group_action_get_commands( current_user_get_accessible_projects() );
30+
$t_action = strtoupper( $p_action );
31+
32+
if ( !isset( $t_valid_actions[$t_action] ) && !isset ( $t_valid_actions['EXT_' . $t_action] ) ) {
33+
trigger_error( ERROR_GENERIC, ERROR );
34+
}
35+
36+
$t_include_file = config_get_global( 'absolute_path' ) . 'bug_actiongroup_' . $p_action . '_inc.php';
37+
if ( !file_exists( $t_include_file ) ) {
38+
trigger_error( ERROR_GENERIC, ERROR );
39+
} else {
40+
require_once( $t_include_file );
41+
}
42+
}
43+
2544
/**
2645
* Print the top part for the bug action group page.
2746
*/
@@ -94,7 +113,6 @@ function bug_group_action_print_hidden_fields( $p_bug_ids_array ) {
94113
* @param $p_action The custom action name without the "EXT_" prefix.
95114
*/
96115
function bug_group_action_print_action_fields( $p_action ) {
97-
require_once( dirname( dirname( __FILE__ ) ) . DIRECTORY_SEPARATOR . 'bug_actiongroup_' . $p_action . '_inc.php' );
98116
$t_function_name = 'action_' . $p_action . '_print_fields';
99117
$t_function_name();
100118
}
@@ -106,7 +124,6 @@ function bug_group_action_print_action_fields( $p_action ) {
106124
* @param $p_action The custom action name without the "EXT_" prefix.
107125
*/
108126
function bug_group_action_print_title( $p_action ) {
109-
require_once( dirname( dirname( __FILE__ ) ) . DIRECTORY_SEPARATOR . 'bug_actiongroup_' . $p_action . '_inc.php' );
110127
$t_function_name = 'action_' . $p_action . '_print_title';
111128
$t_function_name();
112129
}
@@ -121,7 +138,6 @@ function bug_group_action_print_title( $p_action ) {
121138
* @returns true|array true if action can be applied or array of ( bug_id => reason for failure to validate )
122139
*/
123140
function bug_group_action_validate( $p_action, $p_bug_id ) {
124-
require_once( dirname( dirname( __FILE__ ) ) . DIRECTORY_SEPARATOR . 'bug_actiongroup_' . $p_action . '_inc.php' );
125141
$t_function_name = 'action_' . $p_action . '_validate';
126142
return $t_function_name( $p_bug_id );
127143
}
@@ -136,7 +152,6 @@ function bug_group_action_validate( $p_action, $p_bug_id ) {
136152
* @returns true|array Action can be applied., ( bug_id => reason for failure to process )
137153
*/
138154
function bug_group_action_process( $p_action, $p_bug_id ) {
139-
require_once( dirname( dirname( __FILE__ ) ) . DIRECTORY_SEPARATOR . 'bug_actiongroup_' . $p_action . '_inc.php' );
140155
$t_function_name = 'action_' . $p_action . '_process';
141156
return $t_function_name( $p_bug_id );
142157
}

0 commit comments

Comments
 (0)