Add h() for html escaping (Trac #4397)

[elgg-gitbot]

Original ticket http://trac.elgg.org/ticket/4397 on 2012-03-02 by ewinslow, assigned to unknown.

Elgg version: 1.8

htmlspecialchars($text, ENT_QUOTES, 'UTF-8')??? Who wants to type all that?

h($text) is awesome.

[elgg-gitbot]

cash wrote on 2012-03-05

h() is a bad function name.

[elgg-gitbot]

ewinslow wrote on 2012-03-05

Its pretty common to use h for html escaping in templates, in my experience. Why do you think its bad? In terms of descriptive, definitely definitely the exception.

Do you have an alternative?

[elgg-gitbot]

cash wrote on 2012-03-09

I believe one of our goals is to properly namespace Elgg's functions and classes so that external libraries can be used without conflicts.

[elgg-gitbot]

ewinslow wrote on 2012-03-09

if(!function_exists())?

[elgg-gitbot]

cash wrote on 2012-03-09

1. you lose control over functionality when you do that
2. it only works if the library also does a function_exists() call

[elgg-gitbot]

ewinslow wrote on 2012-03-09

Mmm. Good points.

So... alternatives?

[elgg-gitbot]

ewinslow wrote on 2012-04-10

elgg_h?

[elgg-gitbot]

ewinslow wrote on 2012-05-12

elgg_h feels like proper namespacing to me. Slating this for 1.9.

[elgg-gitbot]

Milestone changed to Elgg 1.9.0 by ewinslow on 2012-05-12

[elgg-gitbot]

brettp wrote on 2012-05-12

I really don't like elgg_h(). It has no intrinsic meaning. If we're aiming for brevity, elgg_html() at least carries some meaning as to the function's purpose.

[elgg-gitbot]

cash wrote on 2012-05-14

Agree with Brett on not liking elgg_h(). Why do we care about brevity? I almost never type more than "elgg_xx" to get the function that I want due to function hinting. I prefer elgg_escape().

[elgg-gitbot]

ewinslow wrote on 2012-05-17

It's not just for brevity, but precedent.

[elgg-gitbot]

ewinslow wrote on 2012-05-17

This may be a moot issue because really the only viable way to fight against XSS is by using context-aware, auto-escaped templates, and PHP does not fit that bill.

[elgg-gitbot]

trac user sembrestels wrote on 2012-07-12

We also need cover

htmlspecialchars($text, ENT_NOQUOTES, 'UTF-8');

or

htmlspecialchars($text, ENT_QUOTES, 'UTF-8', false);

(+1 to elgg_escape)

[elgg-gitbot]

cash wrote on 2012-07-13

I think we are going to need context aware escaping functions.

I was thinking something like this:

$title = ElggFormatter::escapeHTML($title);

$attribute = ElggFormatter::escapeAttribute($attribute);

It is more verbose, but I'm ok with that. The attribute escape function understands its context and uses the appropriate escaping which is different from the escaping needed for plain html text.

[elgg-gitbot]

ewinslow wrote on 2012-07-13

Meh. Now I'm getting nervous. We've been down this road at Google and providing escaping functions is basically hopeless. Context-aware template systems are much more effective at actually mitigating the xss attacks.

[elgg-gitbot]

cash wrote on 2012-07-13

Hopeless because it relies on the developers to use them correctly or hopeless for some other reason?

[elgg-gitbot]

ewinslow wrote on 2012-07-13

Relies on devs to use them right.

[elgg-gitbot]

cash wrote on 2012-07-13

Our options for Elgg 1.9 are

1. sprinkle htmlspecialchars() calls through our views with different parameters based on context
2. add some functions that make the context that they should be used in explicit and sprinkle those in our views
3. do nothing
4. rip out the current view system and replace with a context-aware template system
5. use Content Security Policy and do away with all inline CSS/JavaScript

Any other options?

[elgg-gitbot]

ewinslow wrote on 2012-07-13

Use a context aware template system without replacing the current views system.

[elgg-gitbot]

cash wrote on 2012-07-14

Any of the context aware template systems that I have seen in PHP involve a preprocessor that compiles the templates to PHP. That may not involve ripping out the current views system, but does mean rewriting all the views - correct?

[elgg-gitbot]

ewinslow wrote on 2012-07-14

I don't see it being an all or nothing approach. We can convert views to autoescape gradually as we discover vulnerabilities.

[elgg-gitbot]

cash wrote on 2012-07-14

I think we mean different things by autoescaping - sounds like you may mean that we add code to a view so that the input is escaped by default (say for the title field of the module view). I'm talking about a template language other than PHP that gets compiled to PHP with escaping built in.

[elgg-gitbot]

ewinslow wrote on 2012-07-15

We mean the same thing. We convert views to use the template language gradually.