Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-57356

The $https argument for _recaptcha_http_post() not passed correctly

    XMLWordPrintable

    Details

      Description

      This issue originally started while testing the signup procedure at learn.moodle.net. For some reason, the recaptcha did not work. I was able to debug things to what I believe is a core bug.

      Back in 2008, in the issue MDL-14073, commit 83947a36a8c5854f0d9fb5da8f58740069082d22, we introduced a new parameter for the _recaptcha_http_post() function:

      diff --git a/lib/recaptchalib.php b/lib/recaptchalib.php
      index 63374f257d..8b0ed0fb58 100644
      --- a/lib/recaptchalib.php
      +++ b/lib/recaptchalib.php
      @@ -64,8 +64,13 @@ function _recaptcha_qsencode ($data) {
        * @param int port
        * @return array response
        */
      -function _recaptcha_http_post($host, $path, $data, $port = 80) {
      +function _recaptcha_http_post($host, $path, $data, $port = 80, $https=false) {
               global $CFG;
      +        $protocol = 'http';
      +        if ($https) {
      +            $protocol = 'https';
      +        }
      +
               require_once $CFG->libdir . '/filelib.php';
       
               $req = _recaptcha_qsencode ($data);
      

      The new parameter affects whether the Google API servers will be contacted via HTTPS or via HTTP. From what I saw, the intention was that if the Moodle site itself returns is_https() true, it should talk to google via HTTPS too. Note there are relics that try to pass the "https" argument via the mform element contructor explicitly, but that seems to be ignored.

      Anyway, when we call that _recaptcha_http_post() at https://github.com/moodle/moodle/blob/v3.2.0/lib/recaptchalib.php#L240-L248 we do not pass the $https argument correctly. We pass it as the 4th parameter, but it should be the 5th one. The 4th argument $port is ignored.

      Now this apparently did not cause any trouble on most sites. It even works well for us at moodle.org. For some reason, on learn.moodle.org (which itself is running on HTTPS), this caused that cURL did not return any response when calling the Google API via HTTP:

      [Mon Dec 12 17:10:27.099889 2016] [:error] [pid 20199] [client 193.86.4.130:20204] PHP Notice:  Undefined index: http_code in .../lib/filelib.php on line 1352, referer: https://learn.moodle.net/login/signup.php
      [Mon Dec 12 17:10:27.100028 2016] [:error] [pid 20199] [client 193.86.4.130:20204] PHP Notice:  cURL request for "http://www.google.com/recaptcha/api/verify" failed, HTTP response code: Unknown cURL error
      * line 1353 of /lib/filelib.php: call to debugging()
      * line 90 of /lib/recaptchalib.php: call to download_file_content()
      * line 245 of /lib/recaptchalib.php: call to _recaptcha_http_post()
      * line 150 of /lib/form/recaptcha.php: call to recaptcha_check_answer()
      * line 132 of /login/signup_form.php: call to MoodleQuickForm_recaptcha->verify()
      * line 573 of /lib/formslib.php: call to login_signup_form->validation()
      * line 523 of /lib/formslib.php: call to moodleform->validate_defined_fields()
      * line 619 of /lib/formslib.php: call to moodleform->is_validated()
      * line 76 of /login/signup.php: call to moodleform->get_data()
      in .../lib/weblib.php on line 3095, referer: https://learn.moodle.net/login/signup.php
      [Mon Dec 12 17:10:27.100086 2016] [:error] [pid 20199] [client 193.86.4.130:20204] PHP Notice:  Undefined offset: 1 in .../lib/recaptchalib.php on line 257, referer: https://learn.moodle.net/login/signup.php
      

      Fixing the $https argument fixed the signup issue on learn.moodle.net. I don't understand the reasons for this. The site should use same cURL version as moodle.org and has similar setup.

      Still, the way how the argument is passed now is apparently wrong. And we have at least one documented case where it helped (even if i don't really understand why). So even if there is an open issue to migrate to recaptcha v2 (MDL-48501), I am submitting a patch for this.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              Unassigned
              Reporter:
              mudrd8mz David Mudrák (@mudrd8mz)
              Peer reviewer:
              Ankit Agarwal
              Participants:
              Component watchers:
              Andrew Nicols, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze, Amaia Anabitarte, Carlos Escobedo, Ferran Recio, Sara Arjona (@sarjona), Víctor Déniz Falcón
              Votes:
              2 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: