search

ForumPostAssistant / FPA

The Forum Post Assistant (FPA) script has been developed to assist Joomla!® forum posters to be able to post relevant system, instance, PHP and troubleshooting information directly in to a pre-formatted forum post. This will save a few hours of posting back and forth, asking for, and explaining how to acquire useful information in order for other forum users to help troubleshoot a problem.
https://forumpostassistant.github.io/docs/
GNU General Public License v2.0
25 stars 15 forks source link

Include config #45

Closed frostmakk closed 5 years ago

frostmakk commented 6 years ago

Include configuration.php instead of preg_match'ing the settings. https://github.com/orgs/ForumPostAssistant/teams/advisors-reviewers/discussions/15

List the path and filename of unknown extensions manifest files. https://forum.joomla.org/viewtopic.php?f=804&t=963245

fcoulter commented 6 years ago

I have been thinking about this change. I think that in the majority of cases there is no problem with this, however there are some cases where this might cause an issue, if the configuration.php file itself has been hacked:-

  1. If the hack causes a fatal PHP error, then including it will cause the fpa to fail to execute. This is not a particularly unlikely scenario, many hackers are very inept coders and make basic errors;

  2. The hack could introduce malware into the configuration.php file, so including it would execute the malware; I have never actually heard of this happening, but it is possible

Of course usually the read-only permissions make editing configuration.php more difficult, so I think it is rare for it to be hacked, when the index.php is easier, so maybe this is not a big issue.

But perhaps there is more thought required here. Maybe try using preg_match first, then try including configuation.php if this is unsuccessful.

Or do the opposite, but include configuration.php inside a try-catch block, with preg_match used as a fall-back if this fails.

Not sure which is better.

frostmakk commented 6 years ago

As long as Joomla itself include this file, any malware inside will probably have been run many times over before the FPA get the chance to do it.

fcoulter commented 6 years ago

True.

The instance where the configuration.php contains a coding syntax error is probably more of an issue, because that would prevent the fpa from running.

I don't think that either situation is that likely, but still it is worth thinking about.

mandville commented 6 years ago

can this be branched and tested ? ow does this work with j4 changes, aka backward compatible in the future?

frostmakk commented 6 years ago

For testing you can use fpa-en.php in the Config branch. The script is compatible with J4, and all previous versions. The only visible difference is that on/off values are now presented as true/false instead of 1/0. This is also in line with how J4 store the values in configuration.php

frostmakk commented 5 years ago

So, I assume this change has been thoroughly tested the last two months. A couple of thoughts around Fiona's "objections": You can not catch a parse error. That is simply impossible. If, in the forum, we encounter an installation with a faulty config file, there will already be a PHP parse error in the logs. If this is overlooked, and the FPA pre this change is run, it will not reveal the problem, (unless the error is picked up and presented in the report) and people will start looking in the wrong haystacks. With this change merged, (including my last commit) the FPA will on screen produce a parse error, giving a clear indication where the problem is.

mandville commented 5 years ago

I'll merge later after changing versions etc