2014-11-27 04:05 EST

View Issue Details Jump to Notes ] Wiki ]
IDProjectCategoryView StatusLast Update
0013656mantisbtsecuritypublic2014-09-23 18:05
Reporteratrol 
Assigned Torombert 
PriorityurgentSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
Product Version1.2.8 
Target Version1.2.9Fixed in Version1.2.9 
Summary0013656: Reporters have read/write access to existing data of other users
DescriptionI noticed in history that user "a" was able to delete note 10379 in issue 0003932

One of our managers or administrators should check
a) Is the user "a" a reporter?
b) Is configured that reporters are allowed to delete notes?

If a) is true and b) is false the problem might be caused by SOAP API.

Is there any reason why we set $g_mc_readwrite_access_level_threshold = REPORTER; in file mc_config_defaults_inc.php and not DEVELOPER ?

At first sight it seems that a reporter is able to clean up our tracker by using SOAP.
TagsNo tags attached.
Attached Files
  • patch file icon SOAP-API-proper-access-checks-when-deleting-bugs.patch (822 bytes) 2011-12-08 17:51 - 
    From 1af3e6dce4d595be0b13cf6e948ca635ff0aea16 Fri, 9 Dec 2011 00:50:58 +0200
    From: Robert Munteanu <robert.munteanu@gmail.com>
    Date: Fri, 9 Dec 2011 00:49:17 +0200
    Subject: [PATCH] SOAP API: proper access checks when deleting bugs
    
    Affects #13656 : Reporters have read/write access to existing data of other users
    
    diff --git a/api/soap/mc_issue_api.php b/api/soap/mc_issue_api.php
    index 1254039..2be49bb 100644
    --- a/api/soap/mc_issue_api.php
    +++ b/api/soap/mc_issue_api.php
    @@ -943,6 +943,10 @@
     	if( !mci_has_readwrite_access( $t_user_id, $t_project_id ) ) {
     		return mci_soap_fault_access_denied( $t_user_id );
     	}
    +	
    +	if ( !access_has_bug_level( config_get( 'delete_bug_threshold' ), $p_issue_id, $t_user_id ) ) {
    +	    return mci_soap_fault_access_denied( $t_user_id );
    +	}
     
     	return bug_delete( $p_issue_id );
     }
  • patch file icon SOAP-API-proper-access-checks-when-deleting-bugnotes.patch (1,080 bytes) 2011-12-08 17:55 - 
    From 76bb64383a8417f538433770ae6851103cac8f0d Fri, 9 Dec 2011 00:55:06 +0200
    From: Robert Munteanu <robert.munteanu@gmail.com>
    Date: Fri, 9 Dec 2011 00:46:34 +0200
    Subject: [PATCH] SOAP API: proper access checks when deleting bugnotes
    
    Affects #13656 : Reporters have read/write access to existing data of other users
    
    
    diff --git a/api/soap/mc_issue_api.php b/api/soap/mc_issue_api.php
    index 27ae499..1752ba1 100644
    --- a/api/soap/mc_issue_api.php
    +++ b/api/soap/mc_issue_api.php
    @@ -1030,6 +1030,15 @@
     	if( !mci_has_readwrite_access( $t_user_id, $t_project_id ) ) {
     		return mci_soap_fault_access_denied( $t_user_id );
     	}
    +	
    +	$t_reporter_id = bugnote_get_field( $p_issue_note_id, 'reporter_id' );	
    +	
    +	// mirrors check from bugnote_delete.php
    +	if ( ( $t_user_id != $t_reporter_id ) || ( OFF == config_get( 'bugnote_allow_user_edit_delete' ) ) ) {
    +	    if ( !access_has_bugnote_level( config_get( 'delete_bugnote_threshold' ), $p_issue_note_id ) ) {
    +	        return mci_soap_fault_access_denied( $t_user_id );
    +	    }
    +	}
     
     	return bugnote_delete( $p_issue_note_id );
     }

- Relationships
related to 0012328acknowledgedrombert Normalise access checks between the web interface and the SOAP API 
related to 0015721closedgrangeway Functionality to consider porting to master-2.0.x 
related to 0010491closedvboctor mc_issue_add incorrect access level check 
related to 0013736closedrombert mc_issue_get_id_from_summary incorrectly checks for permissions 
related to 0013737closedrombert mci_issue_get_tags_for_bug_id incorrect access checks 
+ Relationships

-  Notes
User avatar

~0030525

atrol (developer)

This was introduced when fixing 0010491
User avatar

~0030527

rombert (developer)

Good catch. I think that - for the time being - we should raise the threshold on mantisbt.org to DEVELOPER immediately.

I will then review the usage of this setting in the SOAP API and ensure that we do not have any security leaks.
User avatar

~0030529

rombert (developer)

Patch to correct access checks when deleting bugs
User avatar

~0030530

rombert (developer)

Patch to correct access check when deleting bugnotes
User avatar

~0030531

rombert (developer)

mc_issue_attachment_get and mc_project_attachment_get still need proper checks, but that requires extracting minimal verification API from file_download.php . Anyway, that's nowhere as severe as the other two issues.
User avatar

~0030536

rombert (developer)

Reminder sent to: dhx, dregad, giallu, grangeway, jreese, siebrand, vboctor

All: this is a pretty serious issue, as all MantisBT installations are vulnerable. I have attached patches for the deletion issues. By monday I should have a proper fix for the 'download unauthorized attachments' issue, as it is still an problem.

I think we should cut a 1.2.9 release to fix this next week. And at the same time advise all users how to appy a hotfix by creating a mc_config_inc.php in api/soap/ and adding

$g_mc_readwrite_access_level_threshold = DEVELOPER;

to it.
User avatar

~0030543

dregad (developer)

Last edited: 2011-12-09 08:27

View 2 revisions

Can one of the admins have a look at the bug history table in the DB, and check if there are more occurences of such deletions ?

If possible, I think it would be useful to restore deleted bugnotes from backup. If that is not possible or we don't have a backup, maybe Google cache can help.

Regarding ~10379 (deleted note in 0003932), I have added the contents of the deleted note into a new one.

User avatar

~0030545

rombert (developer)

I double-checked the access checks for attachment download and they are fine. The only fixes to apply are those attached to the bug.

I'd like to wait pushing them until we are ready to make a release. I will supply fixes for both master and master-1.2.x . The question is, when can we make that release?
User avatar

~0030549

dregad (developer)

I personally don't have any issues with pushing 1.2.9 out.

The fixes I'm currently working on are not so urgent, and can wait for a future release, no problem.

As for the when, I guess that mostly depends on John's availability.
User avatar

~0030637

rombert (developer)

Anyone? Can at least someone with access to the bugtracker set $g_mc_readwrite_access_level_threshold = DEVELOPER as a hotfix until we upgrade the code?
User avatar

~0030643

jreese (reporter)

I've updated this install to set readwrite to developer.
User avatar

~0030806

rombert (developer)

I'm then going to close this with the proposition of releasing 1.2.9 as soon as it is feasible.
User avatar

~0030807

atrol (developer)

Reminder sent to: rombert

You want to release 1.2.9 without applying your patches?
User avatar

~0030808

rombert (developer)

(In reply to comment 0013656:0030807)
> You want to release 1.2.9 without applying your patches?

Thank you. Obviously not. But I'll apply the patches in a few hours.
User avatar

~0030820

atrol (developer)

Reminder sent to: rombert

You didn't push to master-1.2.x
User avatar

~0030821

rombert (developer)

Apparently I didn't . I'll do that tonight.
User avatar

~0030827

rombert (developer)

Third time's the charm, hopefully.

https://github.com/mantisbt/mantisbt/commit/df7782a65e96aa1c9639a7625a658102134c7fe0 [^]
https://github.com/mantisbt/mantisbt/commit/9d3f5783e6e0a4faf4fae13c769c9bfd45bf063c [^]
User avatar

~0030834

dhx (reporter)

Last edited: 2012-01-06 22:25

View 2 revisions

Apologies for missing this bug report until now.

Firstly, thanks for pushing out some fixes.

This is what has always concerned me about MantisBT -- inconsistent permission checks throughout the code base.

The first random SOAP API call I checked is incorrect: mc_issue_get_id_from_summary. It does not take into account whether the issue is private and as a result will always reveal whether or not a private issue with a specified summary exists -- even if the user does not have permission to know about private issues. This is nitpicking because timing analysis of SOAP calls (which we make no attempt to protect against yet) could reveal this information too, so I'm not overly concerned about this one.

The second randomly selected call I checked was also incorrect: mci_issue_get_tags_for_bug_id. This call also fails to check whether the issue is private and whether the user has permission to read the private issue. The access check should be changed to access_has_bug_level instead which correctly handles private issues, project specific permissions, etc.

I don't want to pick on SOAP API because I could open up almost any other file in the MantisBT source tree and find similar problems of inconsistent permission checks. It's really something we need to spend some time sorting out throughout the code base, restructuring to remove these inconsistencies.

User avatar

~0030852

rombert (developer)

David, thanks for the comments ; I've created two new bugs and linked them to this report ; I think that the large security risk was in the deletion problem, but I will fix the other two as well.

I fully agree with your analysis of the current state of security checks - it's too hard because they are currently duplicated across pages and APIs - the SOAP API simply copies what the regular pages are doing.

This is why I've earlier created bug 0012328, which aims to extract a higher-level API usable for all 'clients' which need access to information.
User avatar

~0031386

dhx (reporter)

Last edited: 2012-03-06 11:01

View 2 revisions

This issue is now public to aid with a CVE request on the oss-security mailing list. The latest version (1.2.9) has already been released, including patches to solve this problem.

User avatar

~0031394

dhx (reporter)

A CVE identifier has been assigned to this issue:

CVE-2012-1120 MantisBT 1.2.8 13656
elete_bug_threshold/bugnote_allow_user_edit_delete access check bypass
via SOAP API
User avatar

~0036308

grangeway (reporter)

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

- Issue History
Date Modified Username Field Change
2011-12-08 16:48 atrol New Issue
2011-12-08 16:49 atrol Note Added: 0030525
2011-12-08 17:25 rombert Note Added: 0030527
2011-12-08 17:51 rombert File Added: SOAP-API-proper-access-checks-when-deleting-bugs.patch
2011-12-08 17:51 rombert Note Added: 0030529
2011-12-08 17:55 rombert File Added: SOAP-API-proper-access-checks-when-deleting-bugnotes.patch
2011-12-08 17:55 rombert Note Added: 0030530
2011-12-08 18:25 rombert Note Added: 0030531
2011-12-08 18:25 rombert Priority high => urgent
2011-12-08 18:25 rombert Status new => confirmed
2011-12-08 18:25 rombert Description Updated View Revisions
2011-12-09 04:51 rombert Note Added: 0030536
2011-12-09 08:17 dregad Note Added: 0030543
2011-12-09 08:27 dregad Note Edited: 0030543 View Revisions
2011-12-10 08:29 rombert Note Added: 0030545
2011-12-10 08:29 rombert Assigned To => rombert
2011-12-10 08:29 rombert Status confirmed => assigned
2011-12-10 20:34 dregad Note Added: 0030549
2011-12-16 09:00 rombert Note Added: 0030637
2011-12-16 12:10 jreese Note Added: 0030643
2012-01-05 11:05 rombert Note Added: 0030806
2012-01-05 11:05 rombert Reproducibility have not tried => always
2012-01-05 11:05 rombert Status assigned => resolved
2012-01-05 11:05 rombert Resolution open => fixed
2012-01-05 11:05 rombert Fixed in Version => 1.2.9
2012-01-05 11:50 atrol Note Added: 0030807
2012-01-05 15:19 rombert Note Added: 0030808
2012-01-05 15:19 rombert Status resolved => assigned
2012-01-05 15:19 rombert Resolution fixed => reopened
2012-01-05 15:19 rombert Fixed in Version 1.2.9 =>
2012-01-05 16:47 rombert Status assigned => resolved
2012-01-05 16:47 rombert Resolution reopened => fixed
2012-01-05 18:39 rombert Fixed in Version => 1.2.9
2012-01-06 02:39 atrol Note Added: 0030820
2012-01-06 02:43 rombert Note Added: 0030821
2012-01-06 02:43 rombert Status resolved => assigned
2012-01-06 03:54 rombert Note Added: 0030827
2012-01-06 03:54 rombert Status assigned => resolved
2012-01-06 20:52 dhx Relationship added related to 0010491
2012-01-06 22:24 dhx Note Added: 0030834
2012-01-06 22:25 dhx Note Edited: 0030834 View Revisions
2012-01-07 16:21 rombert Relationship added related to 0013736
2012-01-07 16:22 rombert Relationship added related to 0013737
2012-01-07 16:25 rombert Relationship added related to 0012328
2012-01-07 16:25 rombert Note Added: 0030852
2012-03-03 21:45 vboctor Status resolved => closed
2012-03-06 07:22 dhx Note Added: 0031386
2012-03-06 07:22 dhx View Status private => public
2012-03-06 07:22 dhx Description Updated View Revisions
2012-03-06 11:01 dhx Note Edited: 0031386 View Revisions
2012-03-06 17:34 dhx Note Added: 0031394
2013-04-05 17:57 grangeway Status closed => acknowledged
2013-04-05 17:57 grangeway Note Added: 0036308
2013-04-05 18:25 grangeway Relationship added related to 0015721
2013-04-06 03:42 dregad Status acknowledged => closed
2013-04-06 07:23 grangeway Status closed => 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