Moodle
  1. Moodle
  2. MDL-17931

unable to remove a Moodle Network RPC call if description contains an apostrophe

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.3
    • Fix Version/s: 1.9.8
    • Component/s: MNet
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      31589

      Description

      If I create a custom Moodle Network RPC call, whose description includes an apostrophe, and then remove the call, it fails when trying to remove the record from the database, since it does not properly escape the "help" field:

      You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 's roles from the Moodle system context.',profile = 'a:2:{i:0;a:2:{s:4:"type";s:5' at line 1

      UPDATE mdl_mnet_rpc SET function_name = 'user_get_roles',xmlrpc_path = 'mod/elis/rpclib.php/user_get_roles',parent_type = 'mod',parent = 'elis',enabled = '0',help = 'Returns a user's roles from the Moodle system context.',profile = 'a:2:{i:0;a:2:

      {s:4:"type";s:5:"array";s:11:"description";s:18:"of role shortnames";}

      i:1;s:8:"username";}' WHERE id = 15

      • line 1704 of lib/dmllib.php: call to debugging()
      • line 67 of admin/mnet/adminlib.php: call to update_record()
      • line 145 of admin/mnet/adminlib.php: call to mnet_get_functions()
      • line 466 of admin/index.php: call to upgrade_RPC_functions()

      A possible fix is to edit line 60 and following of admin/mnet/adminlib.php to:
      // Disable functions that don't exist (any more) in the source
      // Should these be deleted? What about their permissions records?
      $rpcrecords = get_records_select('mnet_rpc', ' parent=\''.$parentname.'\' AND parent_type=\''.$type.'\' ', 'function_name ASC ');
      if (!empty($rpcrecords)) {
      foreach($rpcrecords as $rpc) {
      if (!array_key_exists($rpc->function_name, $methodServiceArray))

      { $rec = new stdClass; $rec->id = $rpc->id; $rec->enabled = 0; update_record('mnet_rpc', $rec); }

      }
      }

      1. MDL-17931.patch
        1 kB
        Hubert Chathi
      2. MDL-17931-v2.patch
        0.9 kB
        Penny Leach

        Issue Links

          Activity

          Hide
          Hubert Chathi added a comment -

          In addition, lines 88 and 91 of auth/mnet/adminlib.php are missing calls to addslashes, which means that if documentation for method parameters/return values contain apostrophes, then it fails to insert. Lines 86 through 93 should read:

          array_unshift($profile, $details['returns']);
          }
          $dataobject->profile = addslashes(serialize($profile));
          $dataobject->help = addslashes($details['description']);
          } else

          { $dataobject->profile = addslashes(serialize(array(array('type' => 'void', 'description' => 'No return value')))); $dataobject->help = ''; }
          Show
          Hubert Chathi added a comment - In addition, lines 88 and 91 of auth/mnet/adminlib.php are missing calls to addslashes, which means that if documentation for method parameters/return values contain apostrophes, then it fails to insert. Lines 86 through 93 should read: array_unshift($profile, $details ['returns'] ); } $dataobject->profile = addslashes(serialize($profile)); $dataobject->help = addslashes($details ['description'] ); } else { $dataobject->profile = addslashes(serialize(array(array('type' => 'void', 'description' => 'No return value')))); $dataobject->help = ''; }
          Hide
          Penny Leach added a comment -

          Can you guys attach proper patches if you have them please?

          Show
          Penny Leach added a comment - Can you guys attach proper patches if you have them please?
          Hide
          Hubert Chathi added a comment -

          Yay! MNet is finally getting some attention!

          I've attached a patch for this issue.

          (Now I wish I had some free time to help out with MNet...)

          Show
          Hubert Chathi added a comment - Yay! MNet is finally getting some attention! I've attached a patch for this issue. (Now I wish I had some free time to help out with MNet...)
          Hide
          Penny Leach added a comment -

          Is there a reason you didn't use addslashes_object($dataobject) ? (Just wondering if I'm missing something )

          Show
          Penny Leach added a comment - Is there a reason you didn't use addslashes_object($dataobject) ? (Just wondering if I'm missing something )
          Hide
          Penny Leach added a comment -

          Hmm.. I guess the answer ist hat the rest of that object comes from the request, so it's already slashed.

          Any Petr has already fixed the bit that the bug report was originally about, I'll fix the profile bit too.

          Show
          Penny Leach added a comment - Hmm.. I guess the answer ist hat the rest of that object comes from the request, so it's already slashed. Any Petr has already fixed the bit that the bug report was originally about, I'll fix the profile bit too.
          Hide
          Penny Leach added a comment -

          And why do you need to addslashes this:

          serialize(array(array('type' => 'void', 'description' => 'No return value')));

          Show
          Penny Leach added a comment - And why do you need to addslashes this: serialize(array(array('type' => 'void', 'description' => 'No return value')));
          Hide
          Penny Leach added a comment -

          I think this is enough for the tip of 1.9 - Hubert, what do you think?

          Show
          Penny Leach added a comment - I think this is enough for the tip of 1.9 - Hubert, what do you think?
          Hide
          Hubert Chathi added a comment -

          Yes, I think that should be sufficient. I'm not sure why I added the addslashes to the serialize. I guess it was just being paranoid, in case serialize decided to add random quotes to the string.

          Show
          Hubert Chathi added a comment - Yes, I think that should be sufficient. I'm not sure why I added the addslashes to the serialize. I guess it was just being paranoid, in case serialize decided to add random quotes to the string.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: