Page MenuHomePhabricator

XSS in erwin85 relatedchanges
Closed, ResolvedPublic


Event Timeline

valerio.bozzolan added a project: acl*security.
valerio.bozzolan changed the visibility from "Public (No Login Required)" to "Custom Policy".

Given the security nature of the problem I'm fine with giving permit to WMF security team to have access and fix the problem or if none of maintainers hasn't answered for, let's say, one day. I volunteer to fix it.

I can give you access if you want.

I think in /data/project/erwin85/public_html/inc/


should be replaced to

print_r(array_map("htmlspecialchars", $errcontext),true)

What do you think of htmlentities over htmlspecialchars ?

And assuming a recent enough version of PHP (which, duh) :

print_r( // or var_dump instead of print_r, pretty please ? I strongly advise against print_r, which can give somewhat misleading information in certain situations
        function( $x ) { return htmlentities( $x, ENT_QUOTES | ENT_XHTML, "UTF-8", true ); }, // or htmlspecialchars instead of htmlentities ; besides, let's make sure we call the correct values for those optional params, especially the charset, so it matches the output HTML page, shall we ? :-)
    true // if var_dump(), this line and the trailing comma on the previous line should be removed ; I'm also guessing the echo/print that actually outputs the return value of the print_r() has to be removed

That's off the top of my head (which isn't what it used to be), and without having looked at the code, but it should help a bit :-)

Honestly, i would suggest using htmlspecialchars over htmlentities. Htmlentities does a whole bunch of really pointless stuff.

I'm really OK with both ; but I'd rather leave the lambda function (instead of a plain string callback) in order to keep the parameters set to these explicit values in the call, considering the (X)HTML headers of the output page, if you don't mind.

if($display) {
                        echo '<p><span style="font-weight:bold">' . htmlspecialchars( $type ) . ':</span> ' . htmlspecialchars( $error_msg_html ) . '</p>';

I escaped everything that gets thrown to html.

I also disabled most of the tools. I think i'm gonna delete all of it soon, it seems almost all tools are broken, and for most of them are alternatives. It's time to wind these things down.

I escaped everything that gets thrown to html.

Can I push the change to the public repository? See the git log if you want.

Most of the recursion tools work and are still needed, AFAIK. I'm just not sure any steward or SMWT user is using xwiki.php. Better open a separate task for those deprecations.

Bawolff claimed this task.

I escaped everything that gets thrown to html.

Can I push the change to the public repository? See the git log if you want.

Up to the owner of the tool to decide that sort of policy - but for a toolforge thing, I think its probably easiest.

Anyways, this looks fixed, so closing and making public.

Bawolff changed the visibility from "Custom Policy" to "Public (No Login Required)".Jul 10 2018, 12:14 AM