V3.1/dev issue882

[fzipi]

Fixes issue 882 for v3.1/dev. Should we backport this one?

[csanders-git]

I think this is a big enough change it won't make it into the 3.0.x branches, others thoughts?

[dune73]

We're still discussing it in #882, but I actually think the proposed fix is a bugfix and thus within the scope for backports.

[lifeforms]

I think if it works then we should merge. Any performance hit from introducing extra variables should only be present when somebody submits a http:// parameter and that is pretty rare anyway. It's PL2 also.

[lifeforms]

It has a conflict sadly... @fzipi :'(

[fzipi]

Not anymore! @lifeforms

[lifeforms]

I checked it out and it fixes the bypass partly! The following failing case is FIXED:

http://localhost/?foo=http://evil&bar=http://localhost

However, I found a case (HTTP parameter pollution) is still a bypass after the PR:

http://localhost/?foo=http://evil/&foo=http://localhost/

Note I specified foo parameter twice, the first one is evil, the last one overwrites it and causes it to be bypassed.

Luckily, this may not be a problem for PHP applications, as their parser also gives preference to the last parameter. In the above case, $_GET['foo'] will be set to http://localhost/. But for Java/Python app servers, this can be different as some keep the first match and ignore the last: https://www.owasp.org/index.php/Testing_for_HTTP_Parameter_pollution_(OTG-INPVAL-004)#Expected_Behavior_by_Application_Server (although this data is old)

Could we quickly replace this technique by a parameter counter, as in HTTP parameter pollution rule? Do you think this could be quickly implementable @dune73, I believe you did extensive work on that?

If we can't make a quick fix, I suggest that we merge this PR (which tested fine otherwise and still closes the bypass for many applications), backport it to v3.0/dev, and open a separate issue for the remaining parameter pollution bypass.

[dune73]

Sorry for the long silence. I was teaching abroad for 10 days and really busy between the courses. Now 2 conferences next week and then I'll be free.

As we are intending to backport this, I favor the partial fix.

The bypass is works via HPP. Would not it make sense to accept that as we have a PL3 rule against HPP - or we come to the conclusion that HPP is severe and we shift said rule to PL2.

The HPP was actually pushed by @franbuehler. We wrote the rule together and she did the PR.

For the time being I now plan to follow the suggestion and merge this. Unless I hear opposition soon. ;)

[fzipi]

I concur.

[dune73]

So I merged this. Welcoming backport as discussed. Also new issue.