Moodle
  1. Moodle
  2. MDL-31877

Function get_users function at datalib.php does not remove exceptions

    Details

    • Database:
      Any
    • Testing Instructions:
      Hide

      The $exceptions parameter to get_users isn't actually used anywhere in core that I can see.

      To test you'll need to edit an existing page or create a test page to demonstrate the issue. I've attached a page which demonstrates the fix.

      Show
      The $exceptions parameter to get_users isn't actually used anywhere in core that I can see. To test you'll need to edit an existing page or create a test page to demonstrate the issue. I've attached a page which demonstrates the fix.
    • Workaround:
      Hide

      It can be solved changing on datalib.php in the function get_users:

              //$except = " AND id $exceptions";
      

      for:

              $select .= " AND id $exceptions";
      

      The $except variable is not used in the function

      Show
      It can be solved changing on datalib.php in the function get_users: //$except = " AND id $exceptions" ; for: $select .= " AND id $exceptions" ; The $except variable is not used in the function
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-31877-master-1
    • Rank:
      38522

      Description

      Exceptions does not modifies the $select variable to remove the users selected from the query.

      It could be a security issue if it ignores some users to be assigned as admins for example.

      1. 31877.php
        0.9 kB
        Andrew Nicols

        Activity

        Hide
        Andrew Nicols added a comment -

        This should cherry-pick cleanly to:

        • master
        • MOODLE_22_STABLE
        • MOODLE_21_STABLE
        Show
        Andrew Nicols added a comment - This should cherry-pick cleanly to: master MOODLE_22_STABLE MOODLE_21_STABLE
        Hide
        Jason Fowler added a comment -

        Code looks good, and logical

        Show
        Jason Fowler added a comment - Code looks good, and logical
        Hide
        Andrew Nicols added a comment -

        Integrators: This should cherry-pick cleanly to all stable branches and master

        Show
        Andrew Nicols added a comment - Integrators : This should cherry-pick cleanly to all stable branches and master
        Hide
        Michael de Raadt added a comment -

        Thanks for spotting that and sharing a solution. It should be integrated in the next couple of weeks.

        Show
        Michael de Raadt added a comment - Thanks for spotting that and sharing a solution. It should be integrated in the next couple of weeks.
        Hide
        Pau Ferrer Ocaña (crazyserver) added a comment -

        Happy to read that! Thanks you all!

        Show
        Pau Ferrer Ocaña (crazyserver) added a comment - Happy to read that! Thanks you all!
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Some hours ago...

        the main moodle.git repository has been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Some hours ago... the main moodle.git repository has been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Hide
        Sam Hemelryk added a comment -

        Thanks everyone this has been integrated now and will be available as part of the next releases on all 2.x branches.

        Show
        Sam Hemelryk added a comment - Thanks everyone this has been integrated now and will be available as part of the next releases on all 2.x branches.
        Hide
        Petr Škoda added a comment -

        tested with: $users = get_users(true, '', true, array(1, 3, 2));

        thanks

        Show
        Petr Škoda added a comment - tested with: $users = get_users(true, '', true, array(1, 3, 2)); thanks
        Hide
        Eloy Lafuente (stronk7) added a comment -

        FCT (fixed, closing, thanks). Ciao

        "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!"
        ~ Benjamin Disraeli

        Show
        Eloy Lafuente (stronk7) added a comment - FCT (fixed, closing, thanks). Ciao "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!" ~ Benjamin Disraeli

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: