bgarrels / textpattern

Automatically exported from code.google.com/p/textpattern
0 stars 0 forks source link

Using correct encoding and sanitation methods #299

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Currently Textpattern uses bit strange encoding methods choices for different 
data types. These encoding methods do the most important job which to prevent 
straight out XSS in HTML context, but don't exactly generate proper output.

The function that is thrown almost in any situation is txpspecialchars() (i.e. 
htmlspecialchars()), even in cases where it's not the proper tool.

txpspecialchars() should be used only for HTML documents. For situations such 
as:

- Any HTML attribute value should be prepared with it (where it doesn't cause 
any conflicts).
- Plain-text strings presented in HTML documents.

But excluding:

- query parameters in attributes such as 'src' and 'href'. Those should use 
proper URL encoding OR not encoding at all, leaving the responsibility for the 
use-case.
- Anything JavaScript.

JavaScript strings should be prepared with either:

- escape_js()
- json_encode()

This depends on what the value is for and from where. If the value needs to be 
encoded for HTML too, then the value needs to be passed BOTH txpspecialchars() 
and escape_js()/json_encode().

Of course use of txpspecialchars() is almost never needed since the value is 
for JavaScript and not HTML. JavaScript side should handle the values correctly 
from the start. If there are XSS issue spawning from JavaScript, it should be 
fixed in JavaScript. Not by passing incorrectly encoded values for JavaScript.

URLs:

- URLs, and query strings specifically, for the most part should not be touched 
with none of those. Just urlencoding or nothing. Encoding it for HTML encodes 
it for HTML, not for query string.

Original issue reported on code.google.com by jukka.m.svahn on 26 Oct 2012 at 9:35

GoogleCodeExporter commented 9 years ago
This should be resolved as of r5566. If any new offenders are found, open up a 
new issue.

Original comment by jukka.m.svahn on 25 Aug 2013 at 9:28