Go to main content

Textpattern CMS support forum

You are not logged in. Register | Login | Help

#37 2023-12-18 08:38:54

etc
Developer
Registered: 2010-11-11
Posts: 5,053
Website GitHub

Re: How much should we trust other authors?

phiw13 wrote #336177:

Those did not display/parse with the previous incarnation of the Preview panel (TXP 4.8.8). No changes. No regression.

Body/excerpt preview uses the data currently being edited, not the saved one. I’m not sure txp tags can be safely parsed and previewed in this case, though it’s tempting. Imagine that a bad guy inserts a <txp:php /> block in an admin article body and previews it. The parser will consider that this block comes from an admin article and execute it. Needs mucho thinking.

All <txp: /> tags are handled correctly when viewing the (draft) article in the public side context – even when the “sanitize” checkbox is checked.

‘Sanitize’ acts differently here. It’s just a sandbox CSP rule that silently prevents nasty things from execution, without actually removing/highlighting them. The article is fully parsed first via its page/form, exactly as if it were live. So txp tags are processed, and then the result is displayed in this sandbox (if enabled).

A maybe odd or confusing behaviour with the “sanitize” checkbox under the Save button.

The logic behind this checkbox is that if you were the last to save an article, then it is safe for you, but I’m actually sceptical about its utility. Public side sandboxing is silent (up to some warnings in the browser console), so it does not help to detect nasty things. An admin should inspect/preview the article body/excerpt/cf to be sure that everything is fine.

Offline

#38 2023-12-19 01:15:07

phiw13
Plugin Author
From: Japan
Registered: 2004-02-27
Posts: 3,076
Website

Re: How much should we trust other authors?

etc wrote #336179:

[…] I’m not sure txp tags can be safely parsed and previewed in this case, though it’s tempting. Imagine that a bad guy inserts a <txp:php /> block in an admin article body and previews it. […] Needs mucho thinking.

I see you’ve gone ahead and implemented some of it at least (light, light testing) – in an optional way. Interesting, and a bit frightening, but at least that checkbox is not on (checked) by default.

(and <txp:php /> should not be enabled by default)

The logic behind this checkbox is that if you were the last to save an article, then it is safe for you, but I’m actually sceptical about its utility. Public side sandboxing is silent (up to some warnings in the browser console), so it does not help to detect nasty things. An admin should inspect/preview the article body/excerpt/cf to be sure that everything is fine.

I am not convinced either by that, appart from giving some “feel good” vibes.

An admin should inspect/preview the article body/excerpt/cf to be sure that everything is fine.

This, this 100 times. Which is why I find it useful that the sanitise option on the “Preview” pane does flag those <txp: /> tags.


Where is that emoji for a solar powered submarine when you need it ?
Sand space – admin theme for Textpattern

Offline

#39 2023-12-19 04:55:30

Pat64
Plugin Author
From: France
Registered: 2005-12-12
Posts: 1,599
GitHub Twitter

Re: How much should we trust other authors?

@etc

Your latest commit is perfect: all <txp:... /> tags are parsed in the preview box (even Short Codes)!!


Patrick.

Github | CodePen | Codier | Simplr theme | Wait Me: a maintenance theme | [\a mi.ni.ma]: a “Low Tech” simple Blog theme.

Offline

#40 2023-12-19 08:05:40

etc
Developer
Registered: 2010-11-11
Posts: 5,053
Website GitHub

Re: How much should we trust other authors?

Thank you. It is in a working draft state atm, but looks handy, so I’ll polish it a little later.

phiw13 wrote #336185:

(and <txp:php /> should not be enabled by default)

Yep, also because our user rights are not very tight. As soon as php is enabled, any level 1.2.3 user can upgrade himself to level 1.

Offline

#41 2023-12-19 17:30:47

colak
Admin
From: Cyprus
Registered: 2004-11-20
Posts: 9,011
Website GitHub Mastodon Twitter

Re: How much should we trust other authors?

etc wrote #336176:

They are all potentially unsafe:

<txp:tag label='<script>alert("XSS")</script>' />...

This will not be detected by DOMPurify, since txp is not xml.

If we block all txp tags in the write tab, this means that we will either have to give full privileges to authors or we will have a very limited functionality for multi-author websites.

Authors do not normally have access fo the Presentation which means that <txp:tag label='<script>alert("XSS")</script>' />... is unlikely.

I really believe that shortcodes should be enabled and trust the site owners, together with their designers to decide what they want to allow.

Txp only had a few vulnerabilities in the past 20 years and only a handful of sites have been hacked, mostly because webmasters had WP installed in the same server/site.

At a time that people are asking for more, admittedly wrongly, I do not think that we should be cutting down our existing functionalities.


Yiannis
——————————
NeMe | hblack.art | EMAP | A Sea change | Toolkit of Care
I do my best editing after I click on the submit button.

Offline

#42 2023-12-20 01:18:07

phiw13
Plugin Author
From: Japan
Registered: 2004-02-27
Posts: 3,076
Website

Re: How much should we trust other authors?

@Yiannis,

Nobody is talking about disabling or blocking the <txp: /> tags (including shorttags) for any author level. The discussion in this thread is (only) about the “Preview” widget right there at the top of the main textarea on the Write panel. With Textpattern 4.8 that widget or panel does not parse <txp: /> tags that you may have added to the article body (it only displays the raw tag).

The whole work in this thread is [1] to make that panel more secure (to prevent naughty low level authors doing silly things to eventually hack the account of a higher level user), and [2], as part of this work Oleg has now made it possible to optionally parse and render the content of those tags in the “Preview” panel.

On the public side, nothing changes, authors will still see (preview) their articles will all <txp: /> tags rendered.

–^–

That said, I played a little bit more with that widget yesterday, and the option to view the parsed <txp: /> tags is quite nice. With a long and complex article it took a moment to render, but worked all well.


Where is that emoji for a solar powered submarine when you need it ?
Sand space – admin theme for Textpattern

Offline

#43 2023-12-21 16:07:31

etc
Developer
Registered: 2010-11-11
Posts: 5,053
Website GitHub

Re: How much should we trust other authors?

Please feel free to suggest a finer warning context (only others article? only unpublished yet? only high level users?)

Offline

#44 2023-12-23 04:58:14

Pat64
Plugin Author
From: France
Registered: 2005-12-12
Posts: 1,599
GitHub Twitter

Re: How much should we trust other authors?

Once again, Devs, you have beaten WP! 😃

Stef introduced the new language selector into the login page 15 days before WP.

Etc introduced a parser feature into the preview widget.

https://www.youtube.com/watch?v=6Q6XLGkZO40


Patrick.

Github | CodePen | Codier | Simplr theme | Wait Me: a maintenance theme | [\a mi.ni.ma]: a “Low Tech” simple Blog theme.

Offline

#45 2023-12-24 04:45:32

phiw13
Plugin Author
From: Japan
Registered: 2004-02-27
Posts: 3,076
Website

Re: How much should we trust other authors?

Thinking about the display of “potentially dangerous strings” which is currently done by applying a text-decoration: strikethrough style. As it is it can be confusing, if the article contains <del /> elements (default styling is the same).

Option:

Perhaps the <mark /> element could be used instead (MDN doc)? It is more semantic (highlighting a string). Default styling is a little brilliant (full yellow), but can easily be toned down a little without losing any visual accessibility, and be fully usable in both “light” and “dark” mode, and it has stil has a high colour contrast (7+)

mark { background: hsl(52 88% 77%); color: markText; }

It is not fully accessible, as no AT currently reads it out, see the linked MDN page for some workarounds people have been using. Another eventual problem is that it is an inline element.

Another option:

Use a different/non-default style for strike-through, such as text-decoration: strike-through; text-decoration-style: double or playing with colour (text-decoration-color), but it is more difficult to find a colour that work well, both for “light” and “dark” mode and for contrast. red / hsl(0 99% 50%) / #fc0101 might work, but is slightly difficult for colour blind people (slightly, as they’ll perceive more as black). That could be mixed with text-decoration-thickness:2px.

That is still a problem for AT, but there is currently, as far as I know, no aria-role or HTML element that would fully express what is meant here.

Last edited by phiw13 (2023-12-24 04:46:50)


Where is that emoji for a solar powered submarine when you need it ?
Sand space – admin theme for Textpattern

Offline

#46 2023-12-24 08:21:06

etc
Developer
Registered: 2010-11-11
Posts: 5,053
Website GitHub

Re: How much should we trust other authors?

I gladly leave all this Japanese with you. The preview styles are in textpattern/preview.css now, please feel free to tweak and submit a request.

Offline

Board footer

Powered by FluxBB