Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overly Agressive Template & Plugin Name Validation #15323

Closed
matdave opened this issue Nov 6, 2020 · 1 comment · Fixed by #15349
Closed

Overly Agressive Template & Plugin Name Validation #15323

matdave opened this issue Nov 6, 2020 · 1 comment · Fixed by #15349
Labels
bug The issue in the code or project, which should be addressed.

Comments

@matdave
Copy link
Contributor

matdave commented Nov 6, 2020

Bug report

Summary

The template & plugin name validation introduced in #15146 appears to be overly aggressive and causing issues on older sites. The validation appears to be copy/pasted from chunk and snippet name validation. However, I believe that validation was specifically designed because those elements are "callable" for the templating engine. It is preventing saving of templates on existing sites that use common characters such as colons or apostrophe's.

This also critically breaks Teleport as a template or plugin with a previously valid name can no longer be injected into a site.

Step to reproduce

If you edit a site in the SQL to change the name to something like "Categegory: Interior" you will not be able to save the template within MODX without changing the name.

Observed behavior

This creates confusing behavior as existing Templates are suddenly not capable of being saved without changing the name. It also breaks the ability to inject sites using Teleport if these characters currently exist on their templates or plugins.

Expected behavior

Standard characters that do not pose an issue to XSS or break the site in anyway should be allowed in Template Names and Plugins.

Related issue(s)/PR(s)

Introduced in #15146

Environment

MODX 2.8.1

@garryn
Copy link
Member

garryn commented Nov 6, 2020

Took a look at the validation regex, I'm posting an amendment below for discussion that expands it to include common punctuation I've encountered in Template names (this would also be fine in my opinion to be used for Plugin names):

^(?!\s)[a-zA-Z0-9\x23-\x2f\x3a\x5b-\x5d\x7f-\xff-_\s]+(?<!\s)$

(To see what is included in the various \x ranges, this site is useful: https://www.codetable.net/asciikeycodes)

@JoshuaLuckers JoshuaLuckers added the bug The issue in the code or project, which should be addressed. label Nov 7, 2020
@opengeek opengeek linked a pull request Dec 12, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue in the code or project, which should be addressed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants