Avoid reading form data from query string that produced the form

[charmander]

Note

As a security fix, this is already deployed.

A relatively minor security fix, for form submissions on malicious URLs. Examples:

• /control/editfolder/…?settings=f&settings=m&settings=u&settings=n – add unwanted folder options if owner of folder submits this form

• /control/apikeys?add-api-key=y&add-key-description=%3E:) – add an unintended (but still random and inaccessible) API key with an attacker-controlled description

• revoke OAuth consumers

• add unintended pending collections to accept or reject operations

The problematic pattern is a form with an empty action, preserving the querystring on form submission, combined with the form’s endpoint failing to distinguish between data from the request body and querystring.

The tag filters endpoint wasn’t actually vulnerable as far as I know, but followed the same problematic pattern.

The commission settings forms use formaction attributes.

Any <form>s with empty actions should be revisited eventually (since I’m pretty sure there are only disadvantages to maintaining arbitrary querystrings across form submissions as a default), but more than that, web_input and request.params should be avoided. Fun fact: creating a web.Storage object from request.params.mixed() produces a mapping with the opposite priority compared to request.params itself, i.e. request.params["foo"] will prefer request.GET["foo"] if it exists but request.web_input().foo will prefer request.POST["foo"].