SamJoan / droopescan

A plugin-based scanner that aids security researchers in identifying issues with several CMSs, mainly Drupal & Silverstripe.
GNU Affero General Public License v3.0
1.22k stars 246 forks source link

Drupal: Improvement Request #46

Closed mbomb007 closed 3 years ago

mbomb007 commented 3 years ago

I noticed that for module enumeration you only check for .txt files.

Some of this being used practically can be seen here: https://www.drupalxray.com/

In addition, the "interesting paths" you check aren't that great, as they don't even include the recommended paths for module installation (/sites/all/modules/contrib, /sites/all/modules/custom, /sites/all/modules/features): https://www.drupal.org/node/2621480

SamJoan commented 3 years ago

hi @mbomb007

Lots of good feedback.

  1. Adding readme.md to the module enumeration interesting url enumeration should be trivial, changes required here.
  2. I chose LICENSE.txt as the default module file because I think it's the most commonly found file based on my research. If you think otherwise please let me know.
  3. This seems like a good idea, but would probably require some work.
  4. Adding new file paths: In my experience those directories you mentioned are not used by a large amount of people. If you multiply the number of modules by the number of directories to scan, you get the total number of requests droopescan needs to perform. Roughly doing the math adding those folders would add by default 3000 additional requests. Do you see this as a feasible amount of requests? I think it may be fine.

I am open for a PR for this provided that all tests pass and it's functional. I think most of the changes you are requesting can be fixed by changing the drupal file, except for number three.

With regards to drupalxray and other sites that use droopescan as a service, I will mention that droopescan is licensed under AGPL. People's obligations with respect to that license can be seen https://tldrlegal.com/license/gnu-affero-general-public-license-v3-(agpl-3.0)

If people want to get in touch with me regarding this my email is pedro@worcel.com.

mbomb007 commented 3 years ago

I don't know anything about drupalxray, by the way. I didn't know it used droopescan, but I saw it was similar.

mbomb007 commented 3 years ago

I looked at LICENSE.txt, and it's very common, for sure.

  1. Adding new file paths: In my experience those directories you mentioned are not used by a large amount of people. If you multiply the number of modules by the number of directories to scan, you get the total number of requests droopescan needs to perform. Roughly doing the math adding those folders would add by default 3000 additional requests. Do you see this as a feasible amount of requests? I think it may be fine.

Just a thought: could you detect the first "hit" (checking first by plugin, then by path) and then assume that each type of plugin isn't stored in more than one path? Then, you could stop checking so many paths once you found a valid one.

Or maybe, find a known module, then try multiple possible paths for all of its interesting files, then if nothing is found, you could assume you won't find any others at those paths. When you're guessing and checking, there's all sorts of possibilities to try. shrug

The most promising potential improvement would be to try only valid files for each plugin. So for plugins.txt, you could store an array of files to try for each plugin based on whether the project actually has that file in the repo for any tagged release. You could even limit the requests per plugin to one per type of file by extension.

It'd be more complex to update, but perhaps one could automate it and just add an "optimize" or "update" command that goes and checks the repos and organizes a list of URLs to check. This process could be done once prior to scanning multiple websites.

SamJoan commented 3 years ago

The problem for those assumptions is that we don't know if they'll hold true honestly. In my experience scanning CMS all assumptions about how an installation of a CMS will be broken by developers for one reason or another. For example, they may accidentally install the plugin in two folders, or things like that.

Thinking about it more, 3000 additional requests don't really sound like a lot, especially considering the ever-increasing bandwidth. If you add that PR I'll merge it in as well, and make a release. If anybody has any issues about scanning speed or number of requests in practice they can drop me a line here or elsewhere and we'll revisit.

👍

It would require changes here, keep in mind the trailing slash and %s where the brute-force will happen.

SamJoan commented 3 years ago

Merged your other commit and pushed to PyPi, I tested it and it found some README.md files that it would have missed otherwise, nice work.

SamJoan commented 3 years ago

I think this is good to close right? If not let me know and I'll reopen.

Thanks! Pedro

mbomb007 commented 3 years ago

Yeah, good to close. I'm not actively using the project, so if someone else really wants the additional improvements I thought of, they can submit a pull request.