#1860 closed defect (fixed)

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

Reported by: davidsarah Owned by: David-Sarah Hopwood <david-sarah@…>
Priority: normal Milestone: 1.10.0
Component: code-frontend-web Version: 1.9.2
Keywords: webapi Cc:
Launchpad Bug:


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.

Change History (3)

comment:1 Changed at 2012-11-15T04:17:50Z by David-Sarah Hopwood <david-sarah@…>

  • Owner set to David-Sarah Hopwood <david-sarah@…>
  • Resolution set to fixed
  • Status changed from new to closed

In e097cf96b292f948:

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

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

comment:2 Changed at 2012-11-15T04:20:33Z by davidsarah

  • Summary changed from expansion of %(uri)s in when_done parameter is ill-advised to expansion of %(uri)s in when_done parameter using Python's % operator is ill-advised

comment:3 Changed at 2012-11-15T04:21:33Z by davidsarah

  • Milestone changed from undecided to 1.10.0
Note: See TracTickets for help on using tickets.