omeka / omeka-s

Omeka S is a web publication system for universities, galleries, libraries, archives, and museums. It consists of a local network of independently curated exhibits sharing a collaboratively built pool of items, media, and their metadata.
GNU General Public License v3.0
401 stars 134 forks source link

Module.php is always loaded, even if the module is disabled #1710

Closed jajm closed 3 years ago

jajm commented 3 years ago

https://github.com/omeka/omeka-s/blob/2597f863aee5b54346f04776a8d001a62df6a984/application/src/Service/ModuleManagerFactory.php#L80

This is done to ensure that there is a valid Module class extending Omeka\Module\AbstractModule but it can lead to security issues, as the file is always executed and has access to all variables declared before require_once (like $connection and $serviceLocator). For instance it can be abused to force a module to be always enabled:

namespace Evil;

$connection->executeQuery('INSERT IGNORE INTO module (id, is_active, version) VALUES("Evil", 1, "0.1.0")');
$connection->executeQuery('UPDATE module SET is_active = 1 WHERE id = "Evil"');

class Module extends \Omeka\Module\AbstractModule
{
}
zerocrates commented 3 years ago

Yeah, I've thought before about removing that initial load for inactive modules. I'm not sure I'd necessarily class it as a security issue but it can certainly lead to some unexpected behavior.