U4-8550 - Disallowed Alternate Templates Are Getting Used

Created by Grant Hughes 01 Jun 2016, 15:24:06 Updated by Sebastiaan Janssen 26 Aug 2018, 15:55:01

Tags: Up For Grabs PR

  • Create a brand new Umbraco 7.4.3 site using the default settings such that the Fanoe starter kit is used
  • Once the site is up, navigate to /blogoverview
  • This loads up the home page content item and displays it with the "Blog Overview" template

Because there is no "blogoverview" node, and the home page document type is set to only allow the "Home" template, navigating to /blogoverview should result in a 404. An alternate template should only be usable with a node when the template is marked as allowed on the node's document type.

I have replicated this issue on a 7.3.7 site as well.

Comments

Sebastiaan Janssen 01 Jun 2016, 15:52:44

IMHO it would go too far to support this level of granularity. You can disable altTemplate completely in umbracoSettings.config now. If you still want to use some kind of alternative template for some of the templates then it's up to you to implement that for your own templates, which can be done using route hijacking.


Grant Hughes 01 Jun 2016, 20:14:30

I guess I don't understand your granularity claim. Templates are typically designed around particular document types. Having alternates ignore the allowed template setting yields unpredictable results for pages rendered this way. The current behavior is not at all what I would expect. When choosing that templates A and B are the only ones allowed for my document type, I would not expect the application to happily return the content item with template C.

Based on what I am seeing and your response, it is sounding like the current implementation of the allowed templates feature is that it is a purely administrative feature to limit what templates can be explicitly chosen by the user for a document type. Have I misunderstood anything here because this seems to be misleading to me?


Sebastiaan Janssen 01 Jun 2016, 20:20:02

Granularity: today it's either all altTemplates are allowed or none are allowed.

It pretty easy to go into a template and build in functionality that says: nope, you can't use this as an altTemplate because this template is not an allowed template on this document type. If you want, you can send us a pull request that allows you to configure:

  • All are allowed
  • None are allowed
  • Only templates allowed on this doctype are allowed

We don't currently plan to build this functionality, but we're happy to look at contributions!


Sebastiaan Janssen 01 Jun 2016, 20:23:30

Just as an FYI: this really has never been a problem in all the 8+ years (IIRC) that this functionality has existed. As a website visitor not familiar to the system you'd have to guess what templates are available and try to use them as an altTemplate. We've only ever had people ask: look it's REALLY easy to guess "Home" will be an altTemplate. Can I just disable that behavior, it's annoying.


Grant Hughes 01 Jun 2016, 21:12:42

I understand that this may not be a very common scenario. I did not find other issues about it when I was searching on it.

In my case, this behavior presented itself in two different ways that impacted us. The first was that we found some errors in our logs because people were occasionally somehow navigating to /some-template and the corresponding content item was erroring in that template because the template was not meant for that document type. The second was with how we were doing redirects on the site. We had setup our custom redirects to be checked if no content on the site corresponded to the URL. Here, /whatever matched a template causing a attempted rendering of the unrelated content item with the template. The rendering of the content item prevented our redirect from working. We do make use of the alternates on some of our documents.

Neither of these items are show-stoppers and there are workarounds for them. My logging of this issue is more around how our looking into these problems presented a behavior in Umbraco that we did not expect and that did not seem to make sense.


David Peck 03 Nov 2016, 16:58:29

I think this is an important issue, to avoid any view being called against a model. The result of which is unpredictable, and could result in a serious security issue.

Here is a pull request: https://github.com/umbraco/Umbraco-CMS/pull/1598

This adds a new umbraco config setting disableNotPermittedAlternativeTemplates. Having that enabled will prevent templates in the following situations:

  1. ?altemplate=templateName in the query string
  2. /template name
  3. library.RenderTemplate()

However, I could do with some help testing library.RenderTemplate. I'm not sure how it is supposed to be used but also when I've stuck it on the page it has resulted in some strange behavior.

Additionally, I've not managed to test umbraco.page.cs.

Finally, I've added suitable code to handle this situation in umbraco.SearchForTemplate but I question how this code is used and if it is worth the additional DB call here as I suspect the template is checked again later on in the request.


David Peck 04 Nov 2016, 09:52:00

@sebastiaan - On further reflection, I wonder if the scope of the change as I have suggested is not in keeping with the actual request and is confused.

My view is that the intent of this new setting is to allow developers, who which to allow the URL to select an alternative template, to do so in a controlled manner that prevents a user from entering a URL that results in an unknown result. That is only templates that have been approved through the document type editor can be requested by a user through the URL. This is simply a case of validating input; however, I see a distinction between validating input and putting restrictions on the developer.

Example scenario A blog post can have two different templates (News article and press release), which you would want your editor to be able to select and so only these two templates are set as permissible in the document type editor. However, the website also stores a cookie to identify visually impaired users and a separate set of template is called for these users. The developer needs to apply a different template for these users only and so this template is applied programmatically for the relevant requests. The developer doesn’t want to allow any template to be used. The only issue I see with not restricting the developer is that if a hacker can inject a template alias as part of some form of user input, and it is not validated by the developer, then this could result in an ambiguous result.

Further thoughts Not that I wish to inflate the scope of this issue, but the current setting to prevent alt templates from being used seems to disable the ability to change the template through the URL, but it puts only partial restrictions on the developer. I think it prevents a developer from changing a template in the inbound pipeline and when using library.RenderTemplate, but it doesn’t prevent the developer from returning a ViewResult that relates to alternative template from a controller action. I suggest that the setting should either be a complete security setting stating that unpermitted templates can not be used, or it should prevent the ability to change the template through the URL.

Please let me know your thoughts and I'll send another pull request accordingly.


Grant Hughes 04 Nov 2016, 18:00:34

@david My original reason for this issue was just related to the automatic selection of an alternate template via the URL (?alttemplate=templateName and /templateName) that was not marked as an allowed template for the node type. I was concerned about rendering content and therefore exposing data in an unintended manner due to Umbraco allowing the usage of a template meant for a different node type, as well as the SEO impact of having extra URLs to pages that probably shouldn't be accessible.

I wouldn't think there is a need to restrict anywhere else. A developer should be allowed to choose whatever template they want. If the developer chose the template, then the template should work for the node. If not, that's the developer's problem.


David Peck 08 Nov 2016, 21:17:31

So I've made changes to my branch, which I don't think requires a new PR but please let me know if I'm wrong.

To summarise my solution

A new setting ({{validateAlternativeTemplates}}) is added that validates if a template is 'allowed' (as defined by the document type), when the template is requested through the url in the format /About-us/myTemplateAlias or /About-us?altTemplate=myTemplateAlias.

It does not seek to make any further restrictions on the developer who may assign a different template programmatically.

To prevent this being a breaking change this setting is set to false by default, but it may be worth having the default as false (where not specified) but setting it to true in the umbracoSettings.config (for new installations). Otherwise I would certainly encourage this to be true in V8 as allowing the user control of the view could result in sensitive data being leaked if the developer is exposing sensitive data as a view model, and is reliant on the view not to disclose it.

I'm happy to amend this PR given any further feedback, but otherwise I'm not intending on working on this further.

The related setting {{disableAlternativeTemplates}} will invalidate this new setting, when set to true. Additionally the scope of {{disableAlternativeTemplates}} is greater, currently putting restrictions on {{UmbracoHelper.RenderTemplate()}}, {{Umbraco.Web.library}}, a few locations in the WebForms rendering and other locations.


David Peck 23 Jan 2017, 13:56:49

At the risk of sounding stroppy, is there any reason this hasn't been pulled? I'm happy to change it if it's not right and it was marked as 'up for grabs' so I assume it was wanted?


Jesse Andrews 25 May 2017, 22:01:08

I'm running into this issue with umbraco 7.5.13. The problem is a redirect that maps '''"/about/careers"''' to '''"/careers/."''' "Careers" happens to be a template name, so it renders "/about/careers" with the "Careers" template instead of throwing the expected 404, thus blocking the redirect. When I disable alternate templates with umbracoSettings.config, it prevents the unwanted template from being used, but does '''not''' throw the desired 404 error. Instead it behaves as if '''"/about/"''' was requested instead of '''"/about/careers"'''

It sounds like David's pull request would address this issue, but it hasn't been pulled in yet.


David Peck 22 Feb 2018, 20:28:25

Bump!

If this feature is no longer wanted then I'll take no offence if you want to just close it. If it is wanted then I'm sure there is a good reason why it's not been pulled. If you want it modified then just let me know - happy to help.


Sebastiaan Janssen 26 Aug 2018, 15:55:01

See comment on the PR.. FINALLY!! :-)


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted: Pull request

Affected versions: 7.5.13

Due in version: 7.13.0

Sprint:

Story Points:

Cycle: