Moodle
  1. Moodle
  2. MDL-13992

Global Search requires antiword and pdftotext to be inside the Moodle directory structure

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 1.8.6, 1.9.1
    • Component/s: Global search
    • Labels:
      None
    • Database:
      Any
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE

      Description

      Looking at physical_doc.php and physical_pdf.php, it appears that $CFG->dirroot is applied to the file path, thus causing some real issues for those of use that already have antiword and pdftotext installed. I think that this should be defined in the block config, rather than hardcoded into the file.

      See http://moodle.org/mod/forum/discuss.php?d=92977 for details and a patch (with some corrections in the discussion).

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Valery Fremaux added a comment -

            Hi Matt, my first motivation for forcing moodle to fetch in its own directory an executable for converting files was a security concern :
            would it be damageable or not allowing Moodle executing anything on the system through a configuration value that is held by the database. My first opinion was that any moodle administrator would have ensured that the code he brings back there was secure enough to be reliable, and there would have been no way to get out from there.

            Of course would this force to use a copy of these implementations, or would there be a way to symlink them.

            Anyway, if this might not really constitute a security issue, we could get rid of this path prefixing.

            What is your opinion about this ?
            Thanks

            Show
            Valery Fremaux added a comment - Hi Matt, my first motivation for forcing moodle to fetch in its own directory an executable for converting files was a security concern : would it be damageable or not allowing Moodle executing anything on the system through a configuration value that is held by the database. My first opinion was that any moodle administrator would have ensured that the code he brings back there was secure enough to be reliable, and there would have been no way to get out from there. Of course would this force to use a copy of these implementations, or would there be a way to symlink them. Anyway, if this might not really constitute a security issue, we could get rid of this path prefixing. What is your opinion about this ? Thanks
            Hide
            Matt Campbell added a comment -

            I don't really think it would be a security issue - Moodle already looks outside dirroot and executes items such as du, aspell, and zip - this is defined at Admin->Server->System Paths.

            If you do change this to take out the hardcoded reference to $CFG->dirroot, it will probably break existing installs, so it may Be better to write this so that the global search will look in dirroot AND in the specified filepath.

            Thanks,
            Matt

            Show
            Matt Campbell added a comment - I don't really think it would be a security issue - Moodle already looks outside dirroot and executes items such as du, aspell, and zip - this is defined at Admin->Server->System Paths. If you do change this to take out the hardcoded reference to $CFG->dirroot, it will probably break existing installs, so it may Be better to write this so that the global search will look in dirroot AND in the specified filepath. Thanks, Matt
            Hide
            Valery Fremaux added a comment -

            Hi Matt,

            thanks for your comment. I forecasted your answer already, and was preparing some smoothness within the hard routing to converters.

            Thanks for your advice that I would have to preserve already installed situations. I'll take care of it.

            Cheers.

            Show
            Valery Fremaux added a comment - Hi Matt, thanks for your comment. I forecasted your answer already, and was preparing some smoothness within the hard routing to converters. Thanks for your advice that I would have to preserve already installed situations. I'll take care of it. Cheers.
            Hide
            Valery Fremaux added a comment -

            Follow up of this discussion after implementation of a path switch in global configuration of the search_block

            Show
            Valery Fremaux added a comment - Follow up of this discussion after implementation of a path switch in global configuration of the search_block
            Hide
            Valery Fremaux added a comment -

            Matt,

            Last commit on global search and on block_search should implement now a switch in global search parameters allowing the moodle root prefix to be avoided.

            Please let me know for closing this issue.
            Thanks.

            Show
            Valery Fremaux added a comment - Matt, Last commit on global search and on block_search should implement now a switch in global search parameters allowing the moodle root prefix to be avoided. Please let me know for closing this issue. Thanks.
            Hide
            Valery Fremaux added a comment -

            Added additional Global search parameter for this. Should revise documentation on docs.moodle.org (setup section) .

            Show
            Valery Fremaux added a comment - Added additional Global search parameter for this. Should revise documentation on docs.moodle.org (setup section) .
            Hide
            Mathieu Petit-Clair added a comment -

            Tested on this tuesday-review ... pdftotext correctly gets called. Closed.

            Show
            Mathieu Petit-Clair added a comment - Tested on this tuesday-review ... pdftotext correctly gets called. Closed.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: