tracker issue | What iT iS dESign studios

Title:

encodeFor attribute for cfoutput, writeOutput

| View in Tracker

Status/Resolution/Reason: Closed/Fixed/

Reporter/Name(from Bugbase): David Epler / David Epler (David Epler)

Created: 12/28/2012

Components: Security

Versions: 10.0

Failure Type: Enhancement Request

Found In Build/Fixed In Build: Final /

Priority/Frequency: Trivial / Unknown

Locale/System: English / Platforms All

Vote Count: 4

Listed in the version 2016.0.0.297996 Issues Fixed doc

Verification notes: verified_fixed on September 06, 2019 using build 2016.0.01.298513

While ColdFusion 10 added the various ESAPI encodeFor* functions, it is dependent upon the developer to properly wrap location where used with the appropriate function (e.g. <cfoutput>#EncodeForHTML(url.name)#</cfouput>). Adding an attribute encodeFor negates the need for wrapping individual variables and would process the entire block contained within <cfoutput> for anything within #'s with the appropriate ESAPI EncodeFor* function specified. 

<cfouput encodeFor="HTML">
#url.name#<br>
#url.title#
</cfoutput>

or 

<cfscript>
writeOutput(url.name, "HTML");
</cfscript>

All the ESAPI encoders should be available as attributes: HTML, HTMLAttribute, XML, XMLAttribute, CSS, Javascript, URL

Another example:

<cfouput encodeFor="HTML">
#url.name#
<a href="whatever.cfm?id=#EncodeForURL(url.id)#">Link</a>
</cfoutput>

In the case above the EncodeForURL should take precedence over the EncodeForHTML that would normally occur for just what it is wrapped around. So url.id would be EncodeForURL, while url.name and all other #'s would still be EncodeForHTML.

Default could be set to "none" for backwards compatibility (or "html" for secure profile). Possibly have a server and application level setting to define the default encode. This would make it much easier to deal with blocks of code and provide an easy way to protect against XSS, especially in legacy code bases.

----------------------------- Additional Watson Details -----------------------------

Watson Bug ID:	3434473

External Customer Info:
External Company:  
External Customer Name: David Epler
External Customer Email:

Attachments:

Comments:

+1, that is a good idea! --------------------

Vote by External U.

16840 | January 02, 2013 12:32:34 PM GMT

There should probably be an addition to <cfprocessingdirective> for an encodeFor attribute as well to control the encoding on a per page basis.

Comment by External U.

16820 | January 29, 2013 04:36:37 PM GMT

There definitely needs to be a mechanism for making it easier to write safe code.

Comment by External U.

16821 | February 01, 2013 09:47:20 AM GMT

Would love to see this feature added.  Would make ESAPI much easier.

Vote by External U.

16841 | February 01, 2013 11:39:56 AM GMT

+1, this would help keep code clean and PCI compliant.

Vote by External U.

16842 | March 03, 2013 10:05:13 AM GMT

This would help make our existing code more secure more quickly.

Vote by External U.

16843 | April 18, 2013 05:06:05 PM GMT

Well, it can definitely be a shortcut way of using it but it will reduce the usefulness of cfoutput greatly and hence it would not make much sense to do this. Any HTML tag used in the body of cfoutput will get encoded which I am not sure would be what you would actually want. In your example above, 
<cfouput encodeFor="HTML">
#url.name#<br>
#url.title#
</cfoutput>
<br> will get encoded as "&lt;br&gt;" and you will see <br> as text in the browser.

Comment by Rupesh K.

16822 | June 09, 2013 11:36:41 PM GMT

You did not read the full ticket. I was not talking about encoding the entire <cfoutput> block only things that were inside of #'s

http://www.dcepler.net/post.cfm/better-xss-protection-for-cfml

Also closing as "NotABug" is not appropriate for a feature request.

Comment by External U.

16823 | April 28, 2014 10:56:29 AM GMT

oh ok. Got it. deferring it since it is not possible to take it up for CF 11.

Comment by Rupesh K.

16824 | April 28, 2014 12:33:57 PM GMT

Added a new attribute called encodefor to the output tag. Valid values are html, htmlattribute, url, javascript, css, xml, xmlattribute, xpath, ldap and dn. Based on the value respective encodefor function will be invoked for each expression inside the cfoutput tag body. 

Also the same encoding value can be specified for writeoutput as an argument.

writeoutput(input, encoding)

<cfouput encodeFor="HTML">
#url.name#
<a href="whatever.cfm?id=#EncodeForURL(url.id)#">Link</a>
</cfoutput>

Also, in the above example url.id will be encoded using encodeforurl while the other expression url.name will be encoded using encodeforhtml.

Comment by S V.

16825 | November 19, 2015 12:53:46 AM GMT

I wish I'd seen this before. It sounds a bit couply to me. <cfoutput> and writeOutput() should do *one* job: output.

Encoding should be the remit of encoding functions, not output functions.

This was a bad ER.

Pity it got implemented.

Comment by External U.

16826 | November 20, 2015 04:24:03 AM GMT

Given the following example:

<cfoutput encodeFor="HTML">
#url.name#
<a href="whatever.cfm?id=#url.id#" onclick="get(#url.id#)">Go to whatever.cfm?id=#url.id#</a>
</cfoutput>

Does that mean that all occurrences of #url.id# will be encoded as HTML rather than the correct encoding?

Comment by External U.

16827 | November 20, 2015 06:22:13 AM GMT

Hi John,

Yes, all the occurrences of #url.id# will be encoded using encodeForHTML function. 

<cfoutput encodeFor="HTML">
#url.name#
<a href="whatever.cfm?id=#url.id#" onclick="get(#url.id#)">Go to whatever.cfm?id=#url.id#</a>
</cfoutput>

equivalent code is 

<cfoutput>
#EncodeForHTML(url.name)#
<a href="whatever.cfm?id=#EncodeForHTML(url.id)#" onclick="get(#EncodeForHTML(url.id)#)">Go to whatever.cfm?id=#EncodeForHTML(url.id)#</a>
</cfoutput>

Thanks,
Pavan.

Comment by S V.

16828 | November 20, 2015 06:36:22 AM GMT

Which would be undesirable behaviour. Only #url.name# should be encoded for HTML: the first and third #url.id# should be encodeForURL(), and the second should be encodeForJavaScript().

This just cements for me that this is an ill-conceived ER, and will actually be counterproductive as it will give a false sense of security to developers who don't know what they're doing.

People just need to use the correct function in the correct situation. This is not a hardship.

I recommend cancelling this ER.

Comment by External U.

16829 | November 20, 2015 06:46:09 AM GMT

@Adam,

The proper encoding for the block is:

<cfoutput>
#encodeForHTML(url.name)#
<a href="whatever.cfm?id=#encodeForURL(url.id)#" onclick="get(#encodeForJavascript(url.id))#">Go to whatever.cfm?id=#encodeForHTML(url.id)#</a>
</cfoutput>

So ultimately, I would have preferred that Adobe had put a list of ERs to the PR and said take a look at them and discuss as opposed to a vacuum of the public bugbase where stuff happens randomly, absolutely.

At the end of the day, it is always the developer that has to deal with making the code secure, the original thought behind the ER was to make it a bit easier, but still required understanding of how to use proper encodeFor* functions. Again, it goes to Adobe documenting the change and being extremely explicit that it is not a silver bullet to preventing XSS.

Comment by External U.

16830 | November 20, 2015 08:37:40 AM GMT

Hi Pavan,

Looking at this again, perhaps I misunderstood the ER. It isn't useful (or even correct, as Adam has shown) for all variables to be encoded the same. To be useful, each variable should be properly encoded for its specific context. When all those encodeFor*() functions were introduced, I thought why not just have a way to properly encode all variables for their proper context - instead of peppering a bunch of encodeFor*() functions.

Thanks!,
-Aaron

Comment by External U.

16831 | November 20, 2015 08:44:51 AM GMT

Ah, just now saw David's reply.

Comment by External U.

16832 | November 20, 2015 08:47:50 AM GMT

In regards to the implementation.

writeoutput(input, encoding)

should really be

writeoutput(input, encodeFor)

1) to match the attribute with <cfoutput>
2) "encoding" implies things like UTF-8, iso-8859-1

Comment by External U.

16833 | November 20, 2015 08:49:59 AM GMT

Ok. Since this has been run around as whether this is a good idea or not, there needs to be an understanding as to the origin of the ER.

The task was to remediate security issues in a given ColdFusion application and not refactor it. The said application was monolthic, where .cfm pages are doing everything and there was no framework. So each page looked similar to the following:

<html>
<head>
	<title>...</title>

	<cfoutput>
	<InvalidTag> ... </script>
	</cfoutput>

	<cfoutput>
	<style> ... </style>
	</cfoutput>
</head>
<body>

	<cfoutput>
	(dynamic header)
	</cfoutput>

	<cfoutput>
	(left navigation)
	</cfoutput>

	...

</body>
</html>

Ultimately, the fixes were to put the proper encodeFor*() functions inside the given blocks. What occurred was the idea to add an attribute encodeFor where you could specify the base encoding for the block but still include others which would supersede the attribute so everything was properly encoded.

It was never intended to be a silver bullet nor replace proper understanding of the EncodeFor* functions, but as a faster way to mitigate blocks of <cfoutput> in legacy code.

Comment by External U.

16834 | November 20, 2015 10:26:01 AM GMT

Hi David,

I like the <cfprocessingdirective> idea you and Adam were discussing. Suggestion: <cfprocessingdirective xssprotect="true|false|canonicalize">

Example:

<cfprocessingdirective xssprotect="false"> would be the default and the same as:

<cfoutput>
<style>
  .myClass {color:#myColor#;}
</style>
<div class="#myClass#">
  <a href="foo.cfm?param=#myParam#" onclick="alert('#myAlert#');">#myHTML#</a>
</div>
</cfoutput>

<cfprocessingdirective xssprotect="true"> would be the same as:

<cfoutput>
<style>
  .myClass {color:#encodeForCSS(myColor)#;}
</style>
<div class="#encodeForHTMLAttribute(myClass)#">
  <a href="foo.cfm?param=#encodeForURL(myParam)#" onclick="alert('#encodeForJavaScript(myAlert)#');">#encodeForHTML(myHTML)#</a>
</div>
</cfoutput>

<cfprocessingdirective xssprotect="canonicalize"> would be the same as:

<cfoutput>
<style>
  .myClass {color:#encodeForCSS(myColor, true)#;}
</style>
<div class="#encodeForHTMLAttribute(myClass, true)#">
  <a href="foo.cfm?param=#encodeForURL(myParam, true)#" onclick="alert('#encodeForJavaScript(myAlert, true)#');">#encodeForHTML(myHTML, true)#</a>
</div>
</cfoutput>

I think Adobe can work out if a variable's context is URL, JS, CSS, HTML attribute. If none of those, then it'd use encodeForHTML().

Thanks!,
-Aaron

Comment by External U.

16835 | November 21, 2015 05:13:12 AM GMT

OK, what I think is clear here though is that whilst there might be an ER in here somewhere, the current implementation is not desirable?

David, to be completely honest with you "but as a faster way to mitigate blocks of <cfoutput> in legacy code" should not be the grounds for an ER. You perhaps ought to have written a custom tag and replaced <cfoutput /> with <cf_output /> to solve your immediate issue.  ERs should look forward, not backwards.

Comment by External U.

16836 | November 23, 2015 03:19:46 AM GMT

Hi Adam,

I like the concept (simplifying XSS protection - if Adobe can do it properly), but do agree the current implementation isn't desirable. The [community] discussion has been very good lately; hopefully it's not too late!

Thanks!,
-Aaron

Comment by External U.

16837 | November 23, 2015 04:28:11 AM GMT

To figure out which 'encodefor' function is to be used based on the context will introduce a huge overhead because of the parsing that would be involved, which 'cfoutput' is not expected to do. 
The enhancement is to provide a quick shorthand to encode all the expressions of same type.Also if any other type of expression is present inside cfoutput the respective encodefor function needs to be used.

Comment by S P.

16838 | December 08, 2015 03:27:52 AM GMT

Hi Preethi,

But I was not suggesting cfoutput. I was suggesting <cfprocessingdirective xssprotect="true|false|canonicalize">.

A cfprocessingdirective option, to figure out which encodefor function to use for each context, would be more useful. The result could be cached, so future overhead is reduced. This cfoutput shortcut isn't much of a shortcut b/c one still needs to manually encode all of the -other- contexts.

Thanks!,
-Aaron

Comment by External U.

16839 | December 08, 2015 04:02:14 AM GMT

Hi Adobe,

I've verified this is fixed in CF2016 Update 1 (build 2016.0.01.298513). Well, not optimally b/c CF can't smartly figure out which 'encodefor' function to be used based on context. Regarding the "huge overhead" that'd introduce, we can just cache the page fragments.

Thoughts?

Thanks!,
-Aaron

Comment by Aaron N.

31296 | September 06, 2019 05:32:19 AM GMT

tracker issue : CF-3434473

encodeFor attribute for cfoutput, writeOutput