Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix #14340: Reporters can use SOAP to update bugnotes without permission

The access checks inside bugnote_update.php and
api/soap/mc_issue_api.php differed. Users were incorrectly allowed via
the SOAP interface to update the bugnotes of other users. Instead of
comparing the SOAP user's access level to $g_update_bugnote_threshold,
$g_add_bugnote_threshold was used instead.

This posed a problem because the default installed state of MantisBT is
to allow the REPORTER access level to submit bugs via the SOAP API. Thus
in the default installed state, any user who could submit a bug could
also update/modify the bugnotes of any other user.

Access checks within bugnote_update.php and api/soap/mc_issue_api.php
should now be equivalent.

Thanks to Roland Becker and Damien Regard (both MantisBT developers) for
finding and reporting this problem.
  • Loading branch information...
commit edc8142bb8ac0ac0df1a3824d78c15f4015d959e 1 parent 8e5faf8
@davidhicks davidhicks authored
Showing with 48 additions and 35 deletions.
  1. +48 −35 api/soap/mc_issue_api.php
View
83 api/soap/mc_issue_api.php
@@ -1058,51 +1058,64 @@ function mc_issue_note_delete( $p_username, $p_password, $p_issue_note_id ) {
* @return true on success, false on failure
*/
function mc_issue_note_update( $p_username, $p_password, $p_note ) {
- $t_user_id = mci_check_login( $p_username, $p_password );
-
- if( $t_user_id === false ) {
- return mci_soap_fault_login_failed();
- }
+ $t_user_id = mci_check_login( $p_username, $p_password );
- if ( !isset( $p_note['id'] ) || is_blank( $p_note['id'] ) ) {
- return new soap_fault( 'Client', '', "Issue id must not be blank." );
- }
-
- if ( !isset( $p_note['text'] ) || is_blank( $p_note['text'] ) ) {
- return new soap_fault( 'Client', '', "Issue note text must not be blank." );
- }
-
- $t_issue_note_id = $p_note['id'];
+ if ( $t_user_id === false ) {
+ return mci_soap_fault_login_failed();
+ }
+
+ if ( !isset( $p_note['id'] ) || is_blank( $p_note['id'] ) ) {
+ return new soap_fault( 'Client', '', "Issue id must not be blank." );
+ }
+
+ if ( !isset( $p_note['text'] ) || is_blank( $p_note['text'] ) ) {
+ return new soap_fault( 'Client', '', "Issue note text must not be blank." );
+ }
+
+ $t_issue_note_id = $p_note['id'];
+
+ if ( !bugnote_exists( $t_issue_note_id ) ) {
+ return new soap_fault( 'Server', '', "Issue note '$t_issue_note_id' does not exist." );
+ }
- if( !bugnote_exists( $t_issue_note_id ) ) {
- return new soap_fault( 'Server', '', "Issue note '$t_issue_note_id' does not exist." );
- }
-
$t_issue_id = bugnote_get_field( $t_issue_note_id, 'bug_id' );
-
$t_project_id = bug_get_field( $t_issue_id, 'project_id' );
- if( !mci_has_readwrite_access( $t_user_id, $t_project_id ) ) {
- return mci_soap_fault_access_denied( $t_user_id );
- }
+ 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( 'add_bugnote_threshold' ), $t_issue_id, $t_user_id ) ) {
- return mci_soap_fault_access_denied( $t_user_id, "You do not have access rights to add notes to this issue" );
- }
+ $t_issue_author_id = bugnote_get_field( $t_issue_note_id, 'reporter_id' );
@atrol Collaborator
atrol added a note

This line can be removed.
$t_issue_author_id is not used

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- if( bug_is_readonly( $t_issue_id ) ) {
- return mci_soap_fault_access_denied( $t_user_id, "Issue ' . $t_issue_id . ' is readonly" );
- }
+ # Check if the user owns the bugnote and is allowed to update their own bugnotes
+ # regardless of the update_bugnote_threshold level.
+ $t_user_owns_the_bugnote = bugnote_is_user_reporter( $t_issue_note_id, $t_user_id );
+ $t_user_can_update_own_bugnote = config_get( 'bugnote_allow_user_edit_delete', null, $t_user_id, $t_project_id );
+ if ( $t_user_owns_the_bugnote && !$t_user_can_update_own_bugnote ) {
+ return mci_soap_fault_access_denied( $t_user_id );
+ }
- if( isset( $p_note['view_state'] )) {
- $t_view_state = $p_note['view_state'];
- $t_view_state_id = mci_get_enum_id_from_objectref( 'view_state', $t_view_state );
- bugnote_set_view_state( $t_issue_note_id, $t_view_state_id );
- }
+ # Check if the user has an access level beyond update_bugnote_threshold for the
+ # project containing the bugnote to update.
+ $t_update_bugnote_threshold = config_get( 'update_bugnote_threshold', null, $t_user_id, $t_project_id );
+ if ( !$t_user_owns_the_bugnote && !access_has_bugnote_level( $t_update_bugnote_threshold, $t_user_id, $t_project_id ) ) {
+ return mci_soap_fault_access_denied( $t_user_id );
+ }
+
+ # Check if the bug is readonly
+ if ( bug_is_readonly( $t_issue_id ) ) {
+ return mci_soap_fault_access_denied( $t_user_id, "Issue ' . $t_issue_id . ' is readonly" );
+ }
+
+ if ( isset( $p_note['view_state'] ) ) {
+ $t_view_state = $p_note['view_state'];
+ $t_view_state_id = mci_get_enum_id_from_objectref( 'view_state', $t_view_state );
+ bugnote_set_view_state( $t_issue_note_id, $t_view_state_id );
+ }
- bugnote_set_text( $t_issue_note_id, $p_note['text'] );
+ bugnote_set_text( $t_issue_note_id, $p_note['text'] );
- return bugnote_date_update( $t_issue_note_id );
+ return bugnote_date_update( $t_issue_note_id );
}
/**
@atrol

This line can be removed.
$t_issue_author_id is not used

Please sign in to comment.
Something went wrong with that request. Please try again.