Go to main content

Textpattern CMS support forum

You are not logged in. Register | Login | Help

#1 2023-12-03 14:54:56

bg1
New Member
Registered: 2023-12-01
Posts: 8
Mastodon

Eval of php code in page & article enabled by default

Noticed that function php() in publish/taghandlers does eval() on passed ‘thing’ (allows anything to be eval’ed without further ado) if config parameter
allow_page_php_scripting
or
allow_article_php_scripting
are enabled. Both of these are enabled by default in the txp_prefs settings table.
So, an article with e.g.
<txp:php> phpinfo(); </txp:php>
works by default.

Might be of interest to consider a more defensive approach and to have these params disabled by default, not allowing php code to be eval’ed via articles or pages.

If this is the wrong place to post this, please move/kill.

Offline

#2 2023-12-03 15:05:29

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 11,269
Website GitHub

Re: Eval of php code in page & article enabled by default

Happy to reverse both settings by default on new installs to make it opt-in, unless anyone can think of a reason not to. Or, at minimum, disable the parsing of PHP in articles by default.

Last edited by Bloke (2023-12-03 15:07:01)


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Online

#3 2023-12-04 09:09:07

bg1
New Member
Registered: 2023-12-01
Posts: 8
Mastodon

Re: Eval of php code in page & article enabled by default

Would be interesting to understand how often and under what circumstances PHP code is in use in the content of a) articles and b) pages.

Maybe introducing a small admin snippet that parse all articles, all pages and identifies if PHP code is in use would be helpful,“Analyse content for potential security risks”. When ran it can report the state to the admin including some direct, recommended action. Potentially allowing the admin to send a short, anonymous report to the textpattern development team for statistics.

This could be ran after a system update/upgrade and could be triggered manually by the admin as well.

Result of run could e.g. be:
1) No PHP code found.
a) If params for PHP code eval enabled: “You currently have the configuration to allow PHP code in articles and/or pages to be executed, but you are not using this feature. Do you want to disable PHP code execution in articles/pages (recommended) <yes> <no> buttons.

b) If params for PHP code eval NOT enabled:
- If script ran automatically: No reporting.
- If script ran manually by admin: Report e.g. “All seems ok. No PHP code found in articles/pages and system params for execution of PHP code in articles/pages are disabled. No change to system required.”

2) PHP code found:
a) If params for PHP code eval enabled: “Your articles/pages contain PHP code <more details about which articles/pages listed here> and the system configuration allows for its execution. This is a potential security risk since any kind of PHP colde can be executed this way, including potentially damaging and dangerous code that leads to destruction of your installation or worse, such as leaking information to third parties. It is strongly recommended to disable PHP code execution now. Disable it? <yes> <no> buttons.

b) If params for PHP code eval NOT enabled: Your articles/pages contain PHP code <more details about which articles/pages listed here> but your system configuration does not allow it to be executed. It is recommended to manually remove the PHP code from the articles/pages listed. <ok> button

Offline

#4 2023-12-04 11:57:21

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 11,269
Website GitHub

Re: Eval of php code in page & article enabled by default

I have traditionally included PHP in pages/forms (not necessarily articles), simply because I often write shortcodes that need snippets of PHP. These are, however, becoming fewer and fewer as the tag suite is becoming more powerful. Common use cases were iteration or splitting/combining content, which can mostly be achieved with the break / breakby, and replace global attributes.

So I can certainly see the tipping point (4.9.0, I expect) that PHP snippets are the exception rather than the norm. So maybe now is a great time to flip the defaults.

With regards the check you mentioned, my feeling is that if this functionality is required, a security-minded plugin could offer all that. Turning on/off Prefs from places other than the Preferences panel and showing messages like this is prime plugin territory rather than core. Plus, we try to avoid “phoning home” in core Textpattern. And as we don’t know how much content a site has – I know of some with tens of thousands of articles and loads of complex pages – analysing them may be performance intrusive.

However, adding a simple warning in the (low? high?) Diagnostics output, perhaps in the pre-flight check area, would certainly be something I’d advocate when/if we flip the defaults. The warning could simply be something along the lines of:

You have PHP enabled in your articles/pages. To improve security from potentially malicious and damaging code, consider switching these off via the Preferences panel, unless you absolutely need raw PHP functionality in your site.

Would that work as a compromise?


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Online

#5 2023-12-04 13:25:07

Bloke
Developer
From: Leeds, UK
Registered: 2006-01-29
Posts: 11,269
Website GitHub

Re: Eval of php code in page & article enabled by default

You know what, let’s try it and see what happens. if we need to tweak/revert, we can do so later.


The smd plugin menagerie — for when you need one more gribble of power from Textpattern. Bleeding-edge code available on GitHub.

Txp Builders – finely-crafted code, design and Txp

Online

#6 2023-12-04 16:07:03

bg1
New Member
Registered: 2023-12-01
Posts: 8
Mastodon

Re: Eval of php code in page & article enabled by default

Bloke,

You’re fast.

Thanks for explaining the background and thinking around what you are doing and want to achieve.
Yes on a plugin that can analyse content. Time permitting I can approach that, or someone else may want to do it.
Very good on the 1) flip to have the eval params disabled by default and 2) the diagnose in txp_diag checking if they’re enabled and showing a warning if so.

[ Topic split into new post ]

Offline

Board footer

Powered by FluxBB