How to test all TT2 tags are escaped.

[chrestomanci]

Greetings wise brothers, I seek your wisdom in the matter of how to prevent our carelessness from allowing the forces of darkness to infect our domains with evil so that innocent visitors are not infected with that evil.

Specifically I want to automatically prevent at least one vector for Cross-site_scripting attacks on a website.

Problem:

I am working on a website project, using Dancer with Template Toolkit. The templates are maintained by a separate design team who are not coders, but are good at making the site look aesthetically pleasing.

In order to reduce the chance of XSS on the website we have decided that all that variables interpolated by TT2 must be escaped via a suitable filter (html, html_entity, uri, url or xml). The problem is that people tend to forget to do that when writing templates, and it is not obvious that anything is wrong because for normal input variables the template works fine.

I have already searched the monastery and other places and found that it is not possible to configure TT2 to pass all variables through a filter when interpolating them, so I am looking for some way to check templates to ensure that all variable interpolation includes a suitable filter. Up until now this check has been done manually during code review, but it is tedious and prone to error, so we would like to automate the process. I am hopping to put such a check as a commit trigger in git, and as as unit test on the whole codebase, so that it is hard to commit an unsafe template into git, and if one sneaks in anyway it will get detected by the automatic unit tests and not merged.

Possible solutions:

It looks like it should be fairly straightforward to write an imperfect checking script using basic regular expressions that simply looks for variables in [% %] tags that don't start with a control directive IF, FOREACH etc, or end with | html but that would generate a fair few false errors, and require ongoing maintenance.

Alternatively it looks like I could dig deeply into TT2's parser, compiler and document model to find every instance of where a variable is interpolated, and check if it passes through a filter before appearing in the output. That approach would be much more reliable, but would represent at least a week of work, that I don't want to spend on this particular problem.

Can the monastery suggest a way forward. Is there a testing script or module out there that I have not found?

One thing I can't do is switch to a different templating system that supports automatic filters. I am aware of alternatives such as Text::Xslate, and how they can automatically escape html, are much faster and some even have TT2 like syntax. The problem is that the design department only knows TT2, they don't want to learn anything else, and they will push back hard against any attempt to change.

[moritz]

IMHO the proper solution isn't (or at least not only) rigorous testing, but escaping by default. I've asked about that in the past, Re: HTML::Template vs. Template::Toolkit vs. ?? (esp. the reference to Template::Stash::HTML::Entities and default_escape for Template::Toolkit? might be relevant.

If none of those solutions work well, I'd consider it sufficient reason not to use TT2, and switch to a template system that supports default escaping.

[chrestomanci]

Thanks for your suggestions.

As you say, no support for default escaping ought to be a reason not to use TT2, but unfortunately it is not my decision. The design team are already pushing back against the use of git, code reviews and other modern practices, so there would be no point in starting yet another fight over which templating system to use unless the syntax is identical to TT2 to the point of bug for bug compatibility.

[Rhandom]

I thought we had got AUTO_FILTER into Template::Toolkit. Oh - no, it was that somebody else wrote a module doing just that. A number of years ago on Perlmonks somebody else (not me) asked for the same thing, and then created Template::AutoFilter. I am the author of Template::Alloy (a near drop in for Template::Toolkit) and also added a native configuration item to Template::Alloy called AUTO_FILTER at about this same time. It is pretty easy to use in Alloy, so I assume that it would be easy to use in Template::AutoFilter as well.

use Template::Alloy;
my $t = Template::Alloy->new(AUTO_FILTER=>"html");
$t->process(\qq{[% foo %]\n}, {foo => "(&)"})'

(&)

This excerpt comes from the Template::Alloy pod http://search.cpan.org/~rhandom/Template-Alloy-1.020/lib/Template/Alloy.pod.

AUTO_FILTER

    Can be the name of any filter. Default undef. Any variable returne
+d by a GET directive (including implicit GET) will be passed to the n
+amed filter. This configuration option may also be passed to the CONF
+IG directive.

        # with AUTO_FILTER => 'html'

        [% f = "&"; GET f %] prints &
        [% f = "&"; f %]     prints & (implicit GET)

    If a variable already has another filter applied the AUTO_FILTER i
+s not applied. The "none" scalar virtual method has been added to all
+ow for using variables without reapplying filters.

        # with AUTO_FILTER => 'html'

        [% f = "&";  f | none %] prints &
        [% f = "&"; g = f; g %]  prints &
        [% f = "&"; g = f; g | none %]  prints & (because g = f is a S
+ET directive)
        [% f = "&"; g = GET f; g | none %]  prints & (because the 
+actual GET directive was called)

Update: I realized that this is one solution, but it is not a direct answer to the ops question. See my next reply for the real answer to the ops question.

[Rhandom]

I just realized I didn't answer the question directly. Here is a solution doing what you asked for. It does use some of the deep internals of the Alloy op tree though (so beware). But with a little more tweaking it would be a fine precommit hook - and then you could keep using your existing template system.

#!/usr/bin/env perl

use strict;
use warnings;
use Template::Alloy;

my $file = \ << 'EOF';
[% foo_is_not_filtered %]
[% foo_is_filtered | html %]
[% IF foo %]
   [% bar %]
[% END %]
[% foo_is_none | none %]
[% BLOCK some_block %]
   [% block_var %]
[% END %]
EOF

my $ta  = Template::Alloy->new;
my $doc = $ta->load_template($file);

my $checker;
$checker = sub {
    my $ref = shift;
    return if ! ref $ref;
    foreach my $node (@$ref) {
        next if ! ref $node;
        if ($node->[0] eq 'GET' && $node->[1] =~ /^\d+/) {
            my $expr = $node->[3];
            next if ! ref $expr;
            my $info = $ta->node_info($node);
            if (@$expr < 5 || $expr->[-3] ne '|') {
                print "File \"$info->{'file'}\" line $info->{'line'}: 
+No filtering ($info->{'text'})\n";
            } else {
                next if $expr->[-2] ne 'none'; # comment out to note e
+very filter
                print "File \"$info->{'file'}\" line $info->{'line'}: 
+Used $expr->[-2] filter ($info->{'text'})\n";
            }
        }
        $checker->($_) for @$node;
    }
};

local $ta->{'_component'} = $doc;
$checker->($doc->{'_tree'});


File "input text" line 8: No filtering (block_var)
File "input text" line 1: No filtering (foo_is_not_filtered)
File "input text" line 4: No filtering (bar)
File "input text" line 6: Used none filter (foo_is_none | none)

Update: If there was demand for it, I could certainly look into making this more of a first class method as part of the standard Template::Alloy distribution. Then it could more easily be reused by anybody speaking TT dialects using any TT or TA module.

[chrestomanci]

Thanks, that looks very promising

It looks like I can use your code in a test script to just load each template file in turn, and get a report of any non escaped variables. I can then call that test script from my commit hook and test suite.

How compatible is Template::Alloy with TT2? Seeing as it will only be used for tests it does not have to be perfect, so long as it does not barf and crash over TT2 macros or suchlike.

Thanks a lot.

[Rhandom]

The largest problem with Template::Alloy is directly related to this node - Alloy has more features than Template::Toolkit. This means that in all cases that I have experienced, the imported Template::Toolkit template will work on Template::Alloy. However, because of items like AUTO_FILTER, not necessarily every template in Template::Alloy that uses Alloy only features will work in Template::Toolkit. This is an important item to discuss with your team before moving to Alloy - if you need to maintain cross compatibility, you can still use Alloy but you have to make sure your team only uses the TT2 features. Again - all TT2 templates should work on Alloy (that was one of its design goals).

There is a larger description of what was added here http://search.cpan.org/~rhandom/Template-Alloy-1.020/lib/Template/Alloy/TT.pm.

As much as possible, I always tried to submit patch features to Template::Toolkit to have them implemented. One such feature that actualy made it in was the ~ PRE/POST CHOMP addition (there used to only be =-+). Other features were not accepted because they added to the size of the grammar, and also because the features were decided to be postponed until the release of TT3. As much as possible, I had tried to keep the syntax of Alloy compatible with the planned features of TT3 - but some items were not fully specced in TT3.

If there are items that aren't compatible they are noted in the Template::Alloy:TT pod. The few small language quirks (very minimal parsing edge case discrepancies) are also listed.

Six years and many many billions of page requests served, we are still happy with Alloy. One of the interesting design goals that Alloy helped us solve was that when I first joined my current company - it had templates written in HTML::Template, HTML::Template::Expr, Text::Tmpl, and I wanted to start standarding on Template::Toolkit syntax. After I developed Alloy, for awhile we actually had our headers/footers written in HTML::Template::Expr, and our new content bodies written in TT2 - but all served under the same Template::Alloy engine.

[sundialsvc4]

The OP already says that there is an “eyeball-based” review process that goes into this, and such process will have to remain in place.   I have worked with shops that did use git pre-commit filters, and usually one of two things happened:

• Version-control was end run.
• The git-filter replaced inspection ... if the code passed the filter, it was not examined further.

What did work well ... was a syntax-highlighting filter built into the text editor, in this case Eclipse.   The content of a TT2 macro-sequence was highlighted in one color.   (This much was already available.)   A new custom filter was added which flagged specific error-cases (and added a high-priority message to the editor status-line.)   This made the practice “eyeball evident” for ease of review.   But there was nothing to prevent you from committing anything you wanted to commit, anytime you wanted to commit it, and pushing those changesets to the repo.   The manual eyeball-review of all changesets (in the production or integration branches) still occurred as a mandatory part of the pre-deployment review process.   The highlighting made it considerably easier, though, and it reduced the number of erroneous commits very quickly.   No one wants to “do it wrong.”   An editor highlighting-macro gives subtle and immediate feedback, just as it does when any of us are writing source-code, CSS files, or what-have-you.

[chrestomanci]

Hello sundialsvc4

I use eclipse, and that costom filter looks very interesting. Are you able to post a link or a copy of the source code?

[Anonymous Monk]

Impose a peer-review stage upon the (human) process. The design-department must be required to submit their changes to your department (being the one that is ultimately business-responsible ...), and your department is required to review it. The change does not, cannot, make it into the production environment without your approval. (Business) problem solved.

[wjw]

If there were a guide on "How not to answer a question", this would have to fit in one of the top 10 examples!
• Has nothing to do with Perl
• Does not address the issue
• Exacerbates the problem it claims to solve(Business problem)

---