View Issue Details

IDProjectCategoryView StatusLast Update
0013656mantisbtsecuritypublic2014-12-22 08:22
Reporteratrol 
Assigned Torombert 
PriorityurgentSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version1.2.8 
Target Version1.2.9Fixed in Version1.2.9 
Summary0013656: Reporters have read/write access to existing data of other users
Description

I 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.

Relationships

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

Activities

atrol

atrol

2011-12-08 16:49

developer   ~0030525

This was introduced when fixing 0010491

rombert

rombert

2011-12-08 17:25

developer   ~0030527

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.

rombert

rombert

2011-12-08 17:51

developer   ~0030529

Patch to correct access checks when deleting bugs



SOAP-API-proper-access-checks-when-deleting-bugs.patch (822 bytes)
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 );
 }
rombert

rombert

2011-12-08 17:55

developer   ~0030530

Patch to correct access check when deleting bugnotes



SOAP-API-proper-access-checks-when-deleting-bugnotes.patch (1,080 bytes)
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 );
 }
rombert

rombert

2011-12-08 18:25

developer   ~0030531

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, thats nowhere as severe as the other two issues.

rombert

rombert

2011-12-09 04:51

developer   ~0030536

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.

dregad

dregad

2011-12-09 08:17

developer   ~0030543

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 dont 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.

rombert

rombert

2011-12-10 08:29

developer   ~0030545

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

Id 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?

dregad

dregad

2011-12-10 20:34

developer   ~0030549

I personally dont have any issues with pushing 1.2.9 out.

The fixes Im 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 Johns availability.

rombert

rombert

2011-12-16 09:00

developer   ~0030637

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?

jreese

jreese

2011-12-16 12:10

reporter   ~0030643

Ive updated this install to set readwrite to developer.

rombert

rombert

2012-01-05 11:05

developer   ~0030806

Im then going to close this with the proposition of releasing 1.2.9 as soon as it is feasible.

atrol

atrol

2012-01-05 11:50

developer   ~0030807

Reminder sent to: rombert

You want to release 1.2.9 without applying your patches?

rombert

rombert

2012-01-05 15:19

developer   ~0030808

(In reply to comment 0013656:0030807)

You want to release 1.2.9 without applying your patches?

Thank you. Obviously not. But Ill apply the patches in a few hours.

atrol

atrol

2012-01-06 02:39

developer   ~0030820

Reminder sent to: rombert

You didnt push to master-1.2.x

rombert

rombert

2012-01-06 02:43

developer   ~0030821

Apparently I didnt . Ill do that tonight.

rombert

rombert

2012-01-06 03:54

developer   ~0030827

Third times the charm, hopefully.

https://github.com/mantisbt/mantisbt/commit/df7782a65e96aa1c9639a7625a658102134c7fe0
https://github.com/mantisbt/mantisbt/commit/9d3f5783e6e0a4faf4fae13c769c9bfd45bf063c

dhx

dhx

2012-01-06 22:24

reporter   ~0030834

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 Im 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 dont 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. Its really something we need to spend some time sorting out throughout the code base, restructuring to remove these inconsistencies.

rombert

rombert

2012-01-07 16:25

developer   ~0030852

David, thanks for the comments ; Ive 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 - its 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 Ive earlier created bug 0012328, which aims to extract a higher-level API usable for all clients which need access to information.

dhx

dhx

2012-03-06 07:22

reporter   ~0031386

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.

dhx

dhx

2012-03-06 17:34

reporter   ~0031394

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

grangeway

grangeway

2013-04-05 17:57

reporter   ~0036308

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

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