View Issue Details

IDProjectCategoryView StatusLast Update
0003375mantisbtsecuritypublic2006-12-08 02:38
Reportereiri Assigned Tothraxisp  
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version0.18.0rc1 
Fixed in Version1.1.0a2 
Summary0003375: Bughistory bypasses security on custom fields
Description

I've made some custom fields which should only be seen by the manager of a project.

The bughistory however shows the changes in custom fields, so the contents of these fields can be seen by any user.

Additional Information

Perhaps an option (this would be a feature requet) for showing the bughistory for certain kind of users. This could enhance the performance as well.

TagsNo tags attached.
Attached Files
custom_field_api.patch (820 bytes)   
*** custom_field_api_orig.php	Wed Sep  1 14:29:27 2004
--- custom_field_api.php	Wed Sep  1 14:31:47 2004
***************
*** 570,575 ****
--- 570,591 ----
  	# Data Access
  	#===================================
  
+ 
+     # --------------------
+     # Return the id of a custome field
+     #
+     function custom_field_get_id( $p_field_name ) {
+         $t_custom_field_table = config_get( 'mantis_custom_field_table' );
+         $query = "SELECT id FROM
+                   $t_custom_field_table
+                   WHERE name ='$p_field_name'";
+         $result = db_query( $query );
+  
+         $row = db_fetch_array( $result );
+ 
+         return $row['id'];
+     }
+ 
  	# --------------------
  	# Return an array all custom field ids
  	function custom_field_get_ids( $p_project_id = ALL_PROJECTS ) {
custom_field_api.patch (820 bytes)   
email_api.patch (685 bytes)   
*** email_api_orig.php	Wed Sep  1 14:34:49 2004
--- email_api.php	Wed Sep  1 14:55:21 2004
***************
*** 984,990 ****
  
  		# put history data
  		if ( ON == config_get( 'history_default_visible' )  &&  $t_user_access_level >= config_get( 'view_history_threshold' ) ) {
! 			$t_bug_data['history']  = history_get_raw_events_array( $p_bug_id );
  		}
  
  		# Sponsorship Information
--- 984,990 ----
  
  		# put history data
  		if ( ON == config_get( 'history_default_visible' )  &&  $t_user_access_level >= config_get( 'view_history_threshold' ) ) {
! 			$t_bug_data['history']  = history_get_raw_events_array( $p_bug_id, $p_user_id );
  		}
  
  		# Sponsorship Information
email_api.patch (685 bytes)   
history_api.patch (3,116 bytes)   
*** history_api_orig.php	Wed Sep  1 14:26:35 2004
--- history_api.php	Wed Sep  1 15:20:36 2004
***************
*** 102,108 ****
  	# Retrieves the raw history events for the specified bug id and returns it in an array
  	# The array is indexed from 0 to N-1.  The second dimension is: 'date', 'userid', 'username',
  	# 'field','type','old_value','new_value'
! 	function history_get_raw_events_array( $p_bug_id ) {
  		$t_mantis_bug_history_table	= config_get( 'mantis_bug_history_table' );
  		$t_mantis_user_table		= config_get( 'mantis_user_table' );
  		$t_history_order			= config_get( 'history_order' );
--- 102,108 ----
  	# Retrieves the raw history events for the specified bug id and returns it in an array
  	# The array is indexed from 0 to N-1.  The second dimension is: 'date', 'userid', 'username',
  	# 'field','type','old_value','new_value'
! 	function history_get_raw_events_array( $p_bug_id, $p_user_id = null ) {
  		$t_mantis_bug_history_table	= config_get( 'mantis_bug_history_table' );
  		$t_mantis_user_table		= config_get( 'mantis_user_table' );
  		$t_history_order			= config_get( 'history_order' );
***************
*** 119,141 ****
  				WHERE bug_id='$c_bug_id'
  				ORDER BY date_modified $t_history_order,id";
  		$result = db_query( $query );
! 		$raw_history_count = db_num_rows( $result );
! 		$raw_history = array();
  
! 		for ( $i=0; $i < $raw_history_count; ++$i ) {
! 			$row = db_fetch_array( $result );
  			extract( $row, EXTR_PREFIX_ALL, 'v' );
  
! 			$raw_history[$i]['date']	= db_unixtimestamp( $v_date_modified );
! 			$raw_history[$i]['userid']	= $v_user_id;
  
  			# user_get_name handles deleted users, and username vs realname
! 			$raw_history[$i]['username'] = user_get_name( $v_user_id );
  
! 			$raw_history[$i]['field']		= $v_field_name;
! 			$raw_history[$i]['type']		= $v_type;
! 			$raw_history[$i]['old_value']	= $v_old_value;
! 			$raw_history[$i]['new_value']	= $v_new_value;
  		} # end for loop
  
  		return $raw_history;
--- 119,147 ----
  				WHERE bug_id='$c_bug_id'
  				ORDER BY date_modified $t_history_order,id";
  		$result = db_query( $query );
! 		$count=0;
!         $raw_history = array();
  
!         while($row = db_fetch_array( $result )) {
  			extract( $row, EXTR_PREFIX_ALL, 'v' );
  
!             # make sure the user has read permission to the custom field
!             $v_field_id = custom_field_get_id($v_field_name);
!             if(!empty($v_field_id) && !custom_field_has_read_access($v_field_id, $p_bug_id, $p_user_id)) {
!                     continue;
!             }
! 
! 			$raw_history[$count]['date']	= db_unixtimestamp( $v_date_modified );
! 			$raw_history[$count]['userid']	= $v_user_id;
  
  			# user_get_name handles deleted users, and username vs realname
! 			$raw_history[$count]['username'] = user_get_name( $v_user_id );
  
! 			$raw_history[$count]['field']		= $v_field_name;
! 			$raw_history[$count]['type']		= $v_type;
! 			$raw_history[$count]['old_value']	= $v_old_value;
! 			$raw_history[$count++]['new_value']	= $v_new_value;
!             
  		} # end for loop
  
  		return $raw_history;
history_api.patch (3,116 bytes)   

Relationships

related to 0007345 closedthraxisp Bug history shows records of private notes. 
related to 0007743 closedvboctor Port: CVE-2006-6574 
child of 0004181 closed Features in Mantis 1.1 release 

Activities

jlatour

jlatour

2004-08-08 09:24

reporter   ~0006810

I think this is still an issue with 0.19, but the fix is too complex to be done in time for 0.19.0. Postponing it.

gollum

gollum

2004-09-01 14:26

reporter   ~0007364

I've run into the same problem and fixed most of the problem. I have patches for custom_field_api.php, email_api.php, and history_api.php made against cvs checked out on 8-23-2004. Hope this helps.

Matt_wc

Matt_wc

2004-11-03 19:58

reporter   ~0008243

Just wondering, why is this not in the 0.19 release if there are patches ready to go? (honest question, not trying to be rude)

grangeway

grangeway

2004-11-08 14:02

reporter   ~0008288

Matt, In this case, the patch was added at a point when we were trying to get 0.19 out to users, and needed a 'stop' point. It would be nice to fix every issues but you need to stop somewhere else you'd never release :)

In terms of this patch, although it works, i'm not personally happy with it atm - as far as i can tell from a v. brief look we 'detect' whether it's a custom field by seeing if the fieldname exists as a custom field. One main issue with that:
If your modifying a non-custom field you still get the select id from custom_fields.. DB query. Without reading all the code, i'm thinking that would cause viewing this bug to call an additional 20 Db queries, even though we don't modify any custom fields.

Matt_wc

Matt_wc

2005-03-02 20:37

reporter   ~0009456

grangeway,

Thanks for the comments. I look forward to the 19.3 (or 1.0 RC1).

fcasarra

fcasarra

2006-01-08 12:22

reporter   ~0011887

I have noticed this bug in 1.0.0rc4 also. I suppose it's planned for version 1.1
Also I think is a good idea to allow to configure the visibility of Issue History (and maybe other fields) depending on user privileges.

brunette

brunette

2006-02-01 12:41

reporter   ~0012058

Last edited: 2006-02-01 13:16

I've made a patch for a new bug history.
In fact I've created some new constants for managing custom field in core/constant_inc.php .
and after that the visibility of Issue History (and maybe other fields) depending on user privileges.

-- my patch : mantis-1.0.0rc5-history.tar.gz

becareful email_api.php contains other customizations

polzin

polzin

2006-08-07 08:47

reporter   ~0013212

Last edited: 2006-08-07 08:52

Small remarks:

1) The feature request in the additional info (history only for some users) was implemented in 0.19 with $g_view_history_threshold. (The config_defaults_inc.php claims that it is implemented only for email notification, but I believe that this is not true.)

2) The old patch seems to be rather expensive, because there is a table lookup for each history line (not only for those with custom fields) and no caching is used.

3) The tar file is not so usable, because it is not a patch and contains additional customizations.

My Conclusion: Use $g_view_history_threshold and wait for 1.1. :-)

vboctor

vboctor

2006-08-09 02:16

manager   ~0013235

I had a quick look on mantis-1.0.0rc5-history.tar.gz and noticed the addition of new history changes constants for private entities. I don't think this will work since bugnotes for example can be added as private then moved to public, or the other way around. Do we change the history events when the bugnote visibility gets changed? Do we want to track the change of bug visibility?

polzin

polzin

2006-08-09 03:23

reporter   ~0013237

@vboctor:
I think the solution should go along the lines of the old patch (check history entry visibility when displaying), but caching should be used to reduce the computation time.

Or: Use DHTML to reload the history only when needed. In this case, a brute force implementation of the history entry visibility check would be sufficient, because it would not be called when displaying the issue page.

It would then be as with the filters: Without DHTML the performance is not optimal.

thraxisp

thraxisp

2006-09-24 19:28

reporter   ~0013480

fixed in CVS
core/history_api.php -> 1.35

thraxisp

thraxisp

2006-09-24 19:29

reporter   ~0013482

fixed in CVS
core/history_api.php -> 1.35