expansion of %(uri)s in when_done parameter using Python's % operator is ill-advised

[davidsarah]

At git/src/allmydata/web/unlinked.py@3d771132#L52 we see:

            if "%(uri)s" in redir_to:
                redir_to = redir_to % {"uri": urllib.quote(upload_results.get_uri())
                                         }

This is intended to expand %(uri)s in the when_done parameter of an unlinked upload, to the URI of the new uploaded file.

Python isn't straightforwardly vulnerable to C printf-style format attacks. However, the % operator is still not designed to take untrusted input on the left, and it's a bad idea to use it that way. If nothing else, it is completely undocumentable except by reference to the ​Python format string documentation. Also, any % characters, i.e. URL escapes, in the when_done URL will have to be doubled (encoded as %25%25 in the original URL) so that they are not interpreted as format characters.

It isn't clear that %(uri)s should continue to be supported, but if it is, then it shouldn't be implemented this way.

[David-Sarah Hopwood <david-sarah@…>]

In e097cf96b292f948:

web/unlinked.py: don't use % operator to expand %(uri)s. fixes #1860.

Signed-off-by: David-Sarah Hopwood <david-sarah@…>