ZABBIX BUGS AND ISSUES
  1. ZABBIX BUGS AND ISSUES
  2. ZBX-7693

User type "Zabbix Admin" users can modify the media for all Zabbix users - Security hole

    Details

      Description

      Based on the UI, I would assume (and hope) that only Zabbix Super Admins could modify the media for any user. In the UI, only Zabbix Super Admins can get to the Administration tab to make user changes. Using the API, I did a test today and found that a user of type "Zabbix Admin" user can modify the media for any users in the zabbix system! For history on why I found this, see ZBXNEXT-2122.

      CVE-2014-1685

        Activity

        Hide
        Corey Shaw added a comment -

        ZBX-7693-modify-own-profile.patch fixes the hole by doing two things:

        1. Only Zabbix Super Admins can modify the media for any user.

        2. All other types of users can only modify their own media.

        Show
        Corey Shaw added a comment - ZBX-7693 -modify-own-profile.patch fixes the hole by doing two things: 1. Only Zabbix Super Admins can modify the media for any user. 2. All other types of users can only modify their own media.
        Hide
        Eduards Samersovs added a comment - - edited

        Big thanks for Your patch!

        Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-7693

        Show
        Eduards Samersovs added a comment - - edited Big thanks for Your patch! Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-7693
        Hide
        Pavels Jelisejevs added a comment - - edited

        (1) Zabbix admin users must be able to change media for themselves. You can use user.get with "editable" to check for both existence and permissions to the user.

        Eduards Samersovs RESOLVED r.41893

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (1) Zabbix admin users must be able to change media for themselves. You can use user.get with "editable" to check for both existence and permissions to the user. Eduards Samersovs RESOLVED r.41893 Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (2) Minor typo correction in r41856.

        Eduards Samersovs CLOSED

        Show
        Pavels Jelisejevs added a comment - - edited (2) Minor typo correction in r41856. Eduards Samersovs CLOSED
        Hide
        Pavels Jelisejevs added a comment - - edited

        (3) I'm still able to add new media to other users using user.updatemedia.

        Eduards Samersovs RESOLVED r.41905

        Pavels Jelisejevs The user.updatemedia method must perform all of the permission validation and field validation itself. It should not delegate it to addmedia and deletemedia.

        Eduards Samersovs RESOLVED r.41966

        Pavels Jelisejevs Please refactor the validation code the way we discussed.

        Eduards Samersovs RESOLVED r.42017

        Pavels Jelisejevs I've made changes in r42221, please review.

        Eduards Samersovs OK, Thanks, please review r.42225

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (3) I'm still able to add new media to other users using user.updatemedia. Eduards Samersovs RESOLVED r.41905 Pavels Jelisejevs The user.updatemedia method must perform all of the permission validation and field validation itself. It should not delegate it to addmedia and deletemedia. Eduards Samersovs RESOLVED r.41966 Pavels Jelisejevs Please refactor the validation code the way we discussed. Eduards Samersovs RESOLVED r.42017 Pavels Jelisejevs I've made changes in r42221, please review. Eduards Samersovs OK, Thanks, please review r.42225 Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (4) user.addmedia and user.deletemedia must be validated as well.

        Eduards Samersovs RESOLVED r.41905

        Pavels Jelisejevs

        Regarding user.addmedia:
        1. The CUser::validateAddMedia() must accept the same params as addMedia().
        2. The "foreach ($users as $user) {" loop is unnecessary in CUser::validateAddMedia().
        3. I suggest to change to error message to "You do not have permissions to create media for other users.". It's more correct.

        Regarding user.deletemedia:
        1. The error should be changed to the standard "No permissions to referred object or it does not exist!" message, since we're referencing media, not users in the request.

        Eduards Samersovs RESOLVED r.42017

        Pavels Jelisejevs I've made some changes to deletemedia in r42214, please review.

        Eduards Samersovs OK, please review r.42225

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (4) user.addmedia and user.deletemedia must be validated as well. Eduards Samersovs RESOLVED r.41905 Pavels Jelisejevs Regarding user.addmedia: 1. The CUser::validateAddMedia() must accept the same params as addMedia(). 2. The "foreach ($users as $user) {" loop is unnecessary in CUser::validateAddMedia(). 3. I suggest to change to error message to "You do not have permissions to create media for other users.". It's more correct. Regarding user.deletemedia: 1. The error should be changed to the standard "No permissions to referred object or it does not exist!" message, since we're referencing media, not users in the request. Eduards Samersovs RESOLVED r.42017 Pavels Jelisejevs I've made some changes to deletemedia in r42214, please review. Eduards Samersovs OK, please review r.42225 Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (5) There is a problem with the user.get method that needs to be fixed before we can resolve this issue.

        {
            "editable": true,
            "countOutput": true,
            "userids": [
                "1"
            ]
        }
        

        The request above will always return "1" even if user "1" is not writable for the current user. Due to this bug we can still update media for other users as long as we specify only one user.

        Eduards Samersovs RESOLVED r.41973,41974

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (5) There is a problem with the user.get method that needs to be fixed before we can resolve this issue. { "editable" : true , "countOutput" : true , "userids" : [ "1" ] } The request above will always return "1" even if user "1" is not writable for the current user. Due to this bug we can still update media for other users as long as we specify only one user. Eduards Samersovs RESOLVED r.41973,41974 Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (6) Problems with usermedia.get:

        I'm logged in as an admin user. The following request should return all media for users in my user groups. Now it doesn't return anything.

        {
            "output": "extend"
        }
        

        Adding "editable" to the request must return only my media, but now it returns all of the media.

        Eduards Samersovs RESOLVED r.42017

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (6) Problems with usermedia.get: I'm logged in as an admin user. The following request should return all media for users in my user groups. Now it doesn't return anything. { "output" : "extend" } Adding "editable" to the request must return only my media, but now it returns all of the media. Eduards Samersovs RESOLVED r.42017 Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment -

        TESTED.

        Show
        Pavels Jelisejevs added a comment - TESTED.
        Hide
        Eduards Samersovs added a comment -

        Fixed in versions 2.3.0 (trunk) r.42234, 2.2.2rc1 r.42233

        Show
        Eduards Samersovs added a comment - Fixed in versions 2.3.0 (trunk) r.42234, 2.2.2rc1 r.42233
        Hide
        richlv added a comment - - edited

        (7) this removed translatable string "Cannot insert user media." and added a translatable "DBerror". two problems with that :

        a) it was done during string freeze;
        b) even if the string change is valid, "DBerror" should not be translatable

        Pavels Jelisejevs Fixed directly in 2.2 r42325 and 2.3 r42326.

        Show
        richlv added a comment - - edited (7) this removed translatable string "Cannot insert user media." and added a translatable "DBerror". two problems with that : a) it was done during string freeze; b) even if the string change is valid, "DBerror" should not be translatable Pavels Jelisejevs Fixed directly in 2.2 r42325 and 2.3 r42326.
        Hide
        richlv added a comment - - edited

        (8) this security problem has not been fixed for 1.8 and 2.0 - were they not vulnerable ?

        Pavels Jelisejevs RESOLVED

        • for 2.0 in svn://svn.zabbix.com/branches/dev/ZBX-7693
        • for 1.8 in svn://svn.zabbix.com/branches/dev/ZBX-7693-1.8

        Eduards Samersovs CLOSED

        Show
        richlv added a comment - - edited (8) this security problem has not been fixed for 1.8 and 2.0 - were they not vulnerable ? Pavels Jelisejevs RESOLVED for 2.0 in svn://svn.zabbix.com/branches/dev/ZBX-7693 for 1.8 in svn://svn.zabbix.com/branches/dev/ZBX-7693-1.8 Eduards Samersovs CLOSED
        Hide
        Pavels Jelisejevs added a comment -

        Fixed in 1.8.20rc2 r42354 and 2.0.11rc2 r42358.

        CLOSED.

        Show
        Pavels Jelisejevs added a comment - Fixed in 1.8.20rc2 r42354 and 2.0.11rc2 r42358. CLOSED.
        Hide
        richlv added a comment - - edited

        -------------------------
        Vulnerability description
        -------------------------

        Users of type 'admin' may modify media for other users even though they should be able to modify their own media only.

        Please use CVE-2014-1685 to refer to this vulnerability.

        -------
        Details
        -------

        Users of type 'admin' should be able to modify only their own media. Zabbix API allowed them to modify media for any user.

        This issue has been reported by Corey Shaw.

        -----------------
        Affected versions
        -----------------

        All of the Zabbix versions are vulnerable to this problem.

        --------------
        Fixed versions
        --------------

        These vulnerabilities have been fixed in the latest releases of Zabbix.

        The fix is available in the following Zabbix releases:
        2.2.2
        2.0.11
        1.8.20

        Show
        richlv added a comment - - edited ------------------------- Vulnerability description ------------------------- Users of type 'admin' may modify media for other users even though they should be able to modify their own media only. Please use CVE-2014-1685 to refer to this vulnerability. ------- Details ------- Users of type 'admin' should be able to modify only their own media. Zabbix API allowed them to modify media for any user. This issue has been reported by Corey Shaw. ----------------- Affected versions ----------------- All of the Zabbix versions are vulnerable to this problem. -------------- Fixed versions -------------- These vulnerabilities have been fixed in the latest releases of Zabbix. The fix is available in the following Zabbix releases: 2.2.2 2.0.11 1.8.20

          People

          • Assignee:
            Unassigned
            Reporter:
            Corey Shaw
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: