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.

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.

2004-09-01 14:27

 

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)   

2004-09-01 14:27

 

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)   

2004-09-01 14:27

 

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)   
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

2006-02-01 13:14

 

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

Issue History

Date Modified Username Field Change
2003-10-01 12:33 eiri New Issue
2003-10-02 01:02 vboctor Category bugtracker => security
2003-10-02 01:02 vboctor Product Version => 0.18.0rc1
2004-08-08 09:23 jlatour Status new => acknowledged
2004-08-08 09:23 jlatour Relationship added child of 0004181
2004-08-08 09:24 jlatour Note Added: 0006810
2004-09-01 14:26 gollum Note Added: 0007364
2004-09-01 14:27 gollum File Added: custom_field_api.patch
2004-09-01 14:27 gollum File Added: email_api.patch
2004-09-01 14:27 gollum File Added: history_api.patch
2004-11-03 19:58 Matt_wc Note Added: 0008243
2004-11-07 11:11 grangeway Assigned To => grangeway
2004-11-08 14:02 grangeway Note Added: 0008288
2005-03-02 20:37 Matt_wc Note Added: 0009456
2006-01-08 12:22 fcasarra Note Added: 0011887
2006-01-08 12:23 fcasarra Sponsorship Added fcasarra: US$ 10
2006-01-08 12:23 fcasarra Sponsorship Total 0 => 10
2006-02-01 12:41 brunette Note Added: 0012058
2006-02-01 13:14 brunette File Added: mantis-1.0.0rc5-history.tar.gz
2006-02-01 13:16 brunette Note Edited: 0012058
2006-08-07 08:47 polzin Note Added: 0013212
2006-08-07 08:48 polzin Note Edited: 0013212
2006-08-07 08:52 polzin Note Edited: 0013212
2006-08-09 02:16 vboctor Note Added: 0013235
2006-08-09 03:23 polzin Note Added: 0013237
2006-08-18 00:49 vboctor Relationship added related to 0007345
2006-09-24 19:28 thraxisp Status acknowledged => resolved
2006-09-24 19:28 thraxisp Fixed in Version => 1.1.0a2
2006-09-24 19:28 thraxisp Resolution open => fixed
2006-09-24 19:28 thraxisp Note Added: 0013480
2006-09-24 19:29 thraxisp Note Added: 0013482
2006-09-24 19:31 thraxisp Assigned To grangeway => thraxisp
2006-12-08 02:38 vboctor Status resolved => closed
2007-01-28 18:29 giallu Relationship added related to 0007743