https://bugzilla.wikimedia.org/show_bug.cgi?id=63445

--- Comment #4 from Erik Bernhardson <[email protected]> ---
In includes/TemplateHelper.php
> * getTemplateFilenames() should check for directory traversal

new patch: https://gerrit.wikimedia.org/r/140938

> * progressiveEnhancement() should escape $insertionType and $sectionId

new patch:  https://gerrit.wikimedia.org/r/140940

> * Can you whitelist function names in pipelist, eachPost? 

pipelist turns out to be unused, removed: https://gerrit.wikimedia.org/r/140943

eachPost (and two new block helpers, ifEquals and ifAnonymous) can't be
directly whitelisted.  Basically lightncandy takes this syntax:

    {{#eachPost foo}}
        ...
    {{else}}
        ...
    {{/eachPost}}

The first ... gets converted into a closure as $options['fn'] and the second
... after the {{else}} gets converted into a closure as $options['inv'].  The
block helper then decides based on parameters to call those functions one or
more times.

I suspect your primarily worried about users being able to specify the function
that is called through a tainted variable.  To prevent this i've added a patch
which ensures these callbacks are Closure instances which cannot be directly
provieded by user input.

new patch: https://gerrit.wikimedia.org/r/140944

> * diffRevision: update @param comments to indicate diffContent isn't escaped

@TODO

> * Seems like addReturnTo shouldn't setup the parameters if the request was a 
> POST

@TODO

> * flow_block_header.handlebars and flow_block_header_single_view.handlebars 
> assume revision.content is safe html. flow_preview.handlebars assumes content 
> is safe. It's hard to find where I can prove that-- maybe add a comment in 
> the template to where that is generated, so it's easy to check the 
> correctness?

Matt also noticed this problem,  the following patch makes the decision of to
escape or not escape revision content more specific with less assumptions. 
Perhaps the comments between me and matt on PS2 also make it more clear how
this should work.

new patch: https://gerrit.wikimedia.org/r/140831/

> * form_element's setting the {{tag}} seems like it could be abused. Maybe 
> adding a comment that it really shouldn't be used except in 
> FlowHandlebars.prototype.formElement, and having the default in the switch 
> raise an exception or reset tag to a sane value?

form_element looks to be unused, removed here:
https://gerrit.wikimedia.org/r/140957


There are still two aboved marked as todo, i need to check with another flow
dev about those.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
Wikibugs-l mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l

Reply via email to