Moodle
  1. Moodle
  2. MDL-24561

Forum subscribe.php does not check sesskey()

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.9.9, 2.0
    • Fix Version/s: 1.9.11, 2.0.2
    • Component/s: Forum
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE
    • Rank:
      1150

      Description

      /mod/forum/subscribe.php does not seem to check sesskey(). Therefore, nasty users could use it for CSRF attack and let easily other user to subscribe to many other forums, for example (spam risk).

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          Actually, David, this is a known feature. It is important for the Unsubscribe link in forum emails to work, and that is only possible if sesskey is not required.

          Show
          Tim Hunt added a comment - Actually, David, this is a known feature. It is important for the Unsubscribe link in forum emails to work, and that is only possible if sesskey is not required.
          Hide
          David Mudrak added a comment -

          Well, the link from email should lead to a confirmation page that would, after confirmation, redirect to itself with proper sesskey() value provided. Otherwise, it is pretty easy to create a forum post at moodle.org with a content like:

          <img width="1" height="1" alt="" src="http://moodle.org/mod/forum/subscribe.php?id=1" />
          <img width="1" height="1" alt="" src="http://moodle.org/mod/forum/subscribe.php?id=2" />
          <img width="1" height="1" alt="" src="http://moodle.org/mod/forum/subscribe.php?id=3" />
          <img width="1" height="1" alt="" src="http://moodle.org/mod/forum/subscribe.php?id=4" />
          <img width="1" height="1" alt="" src="http://moodle.org/mod/forum/subscribe.php?id=5" />
          <img width="1" height="1" alt="" src="http://moodle.org/mod/forum/subscribe.php?id=6" />
          ...
          
          Show
          David Mudrak added a comment - Well, the link from email should lead to a confirmation page that would, after confirmation, redirect to itself with proper sesskey() value provided. Otherwise, it is pretty easy to create a forum post at moodle.org with a content like: <img width= "1" height= "1" alt= "" src=" http: //moodle.org/mod/forum/subscribe.php?id=1" /> <img width= "1" height= "1" alt= "" src=" http: //moodle.org/mod/forum/subscribe.php?id=2" /> <img width= "1" height= "1" alt= "" src=" http: //moodle.org/mod/forum/subscribe.php?id=3" /> <img width= "1" height= "1" alt= "" src=" http: //moodle.org/mod/forum/subscribe.php?id=4" /> <img width= "1" height= "1" alt= "" src=" http: //moodle.org/mod/forum/subscribe.php?id=5" /> <img width= "1" height= "1" alt= "" src=" http: //moodle.org/mod/forum/subscribe.php?id=6" /> ...
          Hide
          Tim Hunt added a comment -

          David, perhaps it should, from a security point of view, but I remember Martin talking about this in the past. So, don't change anything here until you have checked with Martin.

          Show
          Tim Hunt added a comment - David, perhaps it should, from a security point of view, but I remember Martin talking about this in the past. So, don't change anything here until you have checked with Martin.
          Hide
          David Mudrak added a comment -

          RFC for Martin and Petr

          Show
          David Mudrak added a comment - RFC for Martin and Petr
          Hide
          Petr Škoda added a comment -

          I agree with David, it should do confirmation dialog when sesskey is not present. We should never do actions like this without sesskey, the usability has to be solved somehow, skipping sesskey protection is not an option.

          Show
          Petr Škoda added a comment - I agree with David, it should do confirmation dialog when sesskey is not present. We should never do actions like this without sesskey, the usability has to be solved somehow, skipping sesskey protection is not an option.
          Hide
          Martin Dougiamas added a comment - - edited

          Yeah makes total sense. So:

          1) If sesskey IS present (perhaps from a link in Moodle) then do what it does now.
          2) If sesskey is NOT present, then show a new confirmation dialog.

          Show
          Martin Dougiamas added a comment - - edited Yeah makes total sense. So: 1) If sesskey IS present (perhaps from a link in Moodle) then do what it does now. 2) If sesskey is NOT present, then show a new confirmation dialog.
          Hide
          David Mudrak added a comment -

          I just created PULL-75 (for Moodle 2.0) and PULL-76 (for Moodle 1.9) with the patches of this.

          Instructions for QA testers
          ================================

          Moodle 2.0:
          1. Make sure that "Subscribe to this forum" and "Unsubscribe from this forum" links at the front page for "Site news" work and that the link URL contains &sesskey=xxx parameter
          2. Test that if you copy URL of that link, paste it into the address bar of your browser and manually delete the &sesskey=xxx parameter, a confirmation page appears. That is how (un)subscribe from email links will work
          Go to a course and there into a News forum (or any other forum). Check the Settings block
          3. Make sure that "Subscribe to this forum" and "Unsubscribe from this forum" work. Test that if the &sesskey=xxx parameter is missing, the confirmation page appears
          4. Test that the links that modify subscription mode of the forum (Optional subscription, Forced subscription, etc.) work and that their URL contains &sesskey=xxx parameter
          5. Test that these subscription mode change links throws error if &sesskey= param is missing

          Moodle 1.9:
          1. Make sure that "Subscribe to this forum" and "Unsubscribe from this forum" links at the front page for "Site news" work and that the link URL contains &sesskey=xxx parameter
          2. Test that if you copy URL of that link, paste it into the address bar of your browser and manually delete the &sesskey=xxx parameter, a confirmation page appears. That is how (un)subscribe from email links will work
          Go to a course and there into a News forum (or any other forum). Check the top right corner of the screen with subscription settings
          3. Make sure that "Subscribe to this forum" and "Unsubscribe from this forum" work. Test that if the &sesskey=xxx parameter is missing, the confirmation page appears
          4. Test that the links that modify forced subscription mode of the forum (Force everyone to be subscribed and Allow everyone to choose) work and that their URL contains &sesskey=xxx parameter
          5. Test that these subscription mode change links throws error if &sesskey= param is missing
          Go to a page that lists all forums in the course (via the breadcrumb navigation)
          6. Test that "Subscribe to all forums" and "Unsubscribe from all forums" work and that they require &sesskey=xxx parameter (they must throw error if the parameter is missing).

          Note: As a side effect of the patch, I also fixed another buggy behaviour - now administrators and managers can change forum subscription mode even if they are not enrolled in the course. That was broken in 2.0 as a regression of is_guest() replacement with is_enrolled(). Feel free to test that, too.

          Show
          David Mudrak added a comment - I just created PULL-75 (for Moodle 2.0) and PULL-76 (for Moodle 1.9) with the patches of this. Instructions for QA testers ================================ Moodle 2.0: 1. Make sure that "Subscribe to this forum" and "Unsubscribe from this forum" links at the front page for "Site news" work and that the link URL contains &sesskey=xxx parameter 2. Test that if you copy URL of that link, paste it into the address bar of your browser and manually delete the &sesskey=xxx parameter, a confirmation page appears. That is how (un)subscribe from email links will work Go to a course and there into a News forum (or any other forum). Check the Settings block 3. Make sure that "Subscribe to this forum" and "Unsubscribe from this forum" work. Test that if the &sesskey=xxx parameter is missing, the confirmation page appears 4. Test that the links that modify subscription mode of the forum (Optional subscription, Forced subscription, etc.) work and that their URL contains &sesskey=xxx parameter 5. Test that these subscription mode change links throws error if &sesskey= param is missing Moodle 1.9: 1. Make sure that "Subscribe to this forum" and "Unsubscribe from this forum" links at the front page for "Site news" work and that the link URL contains &sesskey=xxx parameter 2. Test that if you copy URL of that link, paste it into the address bar of your browser and manually delete the &sesskey=xxx parameter, a confirmation page appears. That is how (un)subscribe from email links will work Go to a course and there into a News forum (or any other forum). Check the top right corner of the screen with subscription settings 3. Make sure that "Subscribe to this forum" and "Unsubscribe from this forum" work. Test that if the &sesskey=xxx parameter is missing, the confirmation page appears 4. Test that the links that modify forced subscription mode of the forum (Force everyone to be subscribed and Allow everyone to choose) work and that their URL contains &sesskey=xxx parameter 5. Test that these subscription mode change links throws error if &sesskey= param is missing Go to a page that lists all forums in the course (via the breadcrumb navigation) 6. Test that "Subscribe to all forums" and "Unsubscribe from all forums" work and that they require &sesskey=xxx parameter (they must throw error if the parameter is missing). Note: As a side effect of the patch, I also fixed another buggy behaviour - now administrators and managers can change forum subscription mode even if they are not enrolled in the course. That was broken in 2.0 as a regression of is_guest() replacement with is_enrolled(). Feel free to test that, too.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: