Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix #14016: delete_attachments_threshold is not checked

Roland Becker (MantisBT developer) reported the following
security/access control bug:

In a default installation delete_attachments_threshold is set to
DEVELOPER but having access level >= update_bug_threshold is enough to
delete attachments if form_security_validation is set to OFF.

MantisBT was not checking the access level of the user requesting
deletion of an attachment to an issue against
$g_delete_attachments_threshold.

The new access control logic for deleting an issue attachment is now:
1. Does the user have an access level of at least update_bug_threshold?
2. If the user is the owner of the file and
$g_allow_delete_own_attachments=OFF, does this user have an access level
of at least delete_attachments_threshold?
3. If the user is not the owner of the file, do they have an access
level of at least delete_attachments_threshold?

Also refer to issue #14015 for discussion on whether
update_bug_threshold should be part of the access control logic.

The relevant SOAP API call has also been updated.
  • Loading branch information...
commit ceafe6f0c679411b81368052633a63dd3ca06d9c 1 parent 804f6ed
@davidhicks davidhicks authored
Showing with 21 additions and 0 deletions.
  1. +15 −0 api/soap/mc_issue_attachment_api.php
  2. +6 −0 bug_file_delete.php
View
15 api/soap/mc_issue_attachment_api.php
@@ -64,12 +64,27 @@ function mc_issue_attachment_add( $p_username, $p_password, $p_issue_id, $p_name
*/
function mc_issue_attachment_delete( $p_username, $p_password, $p_issue_attachment_id ) {
$t_user_id = mci_check_login( $p_username, $p_password );
+
if( $t_user_id === false ) {
return mci_soap_fault_login_failed();
}
+
$t_bug_id = file_get_field( $p_issue_attachment_id, 'bug_id' );
+
+ # Check access against update_bug_threshold
if( !access_has_bug_level( config_get( 'update_bug_threshold' ), $t_bug_id, $t_user_id ) ) {
return mci_soap_fault_access_denied( $t_user_id );
}
+
+ $t_attachment_owner = file_get_field( $f_file_id, 'user_id' );
@atrol Collaborator
atrol added a note

$f_file_id is not defined at this place.
Should be $p_issue_attachment_id

I fixed it with commit c931418

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $t_current_user_is_attachment_owner = $t_attachment_owner == $t_user_id;
+ # Factor in allow_delete_own_attachments=ON|OFF
+ if ( !$t_current_user_is_attachment_owner || ( $t_current_user_is_attachment_owner && !config_get( 'allow_delete_own_attachments' ) ) ) {
+ # Check access against delete_attachments_threshold
+ if ( !access_has_bug_level( config_get( 'delete_attachments_threshold' ), $t_bug_id, $t_user_id ) ) {
+ return mci_soap_fault_access_denied( $t_user_id );
+ }
+ }
+
return file_delete( $p_issue_attachment_id, 'bug' );
}
View
6 bug_file_delete.php
@@ -44,6 +44,12 @@
access_ensure_bug_level( config_get( 'update_bug_threshold' ), $t_bug_id );
+ $t_attachment_owner = file_get_field( $f_file_id, 'user_id' );
+ $t_current_user_is_attachment_owner = $t_attachment_owner == auth_get_current_user_id();
+ if ( !$t_current_user_is_attachment_owner || ( $t_current_user_is_attachment_owner && !config_get( 'allow_delete_own_attachments' ) ) ) {
+ access_ensure_bug_level( config_get( 'delete_attachments_threshold'), $t_bug_id );
+ }
+
helper_ensure_confirmed( lang_get( 'delete_attachment_sure_msg' ), lang_get( 'delete_attachment_button' ) );
file_delete( $f_file_id, 'bug' );
Please sign in to comment.
Something went wrong with that request. Please try again.