Skip to content

OD703 markup-injection-flow

Data a user controls is wrapped in Markup() without escaping.

Category Severity Series Needs the index
Security Warning all no

What it reports

A value from a source that reaches Markup(), directly or through formatting, string methods, locals and the methods it is passed to. Markup("<p>%s</p>") % value and Markup("<p>{}</p>").format(value) escape the value and are not reported, nor is a value passed through escape(), html_escape(), html_sanitize() or plaintext2html().

Why it matters

Markup marks a string as safe HTML, so QWeb's t-out and the web client render it as it is. Text a user typed then runs as script in the browser of whoever views the page (cross-site scripting), with that user's session. An Html field cleans what is written to it unless it sets sanitize=False, so rich text that only goes into such a field is safe; a Markup value rendered or returned on the way, or stored in an unsanitized field, is not cleaned. The finding asks for that to be confirmed, or for the value to be escaped.

Example

controllers/main.py
class FeedbackController(http.Controller):
    @http.route("/feedback/preview", type="http", auth="public")
    def preview(self, comment):
        html = Markup(comment.replace("\n", "<br/>"))
        return request.render("feedback.preview", {"comment": html})

A <script> in comment runs in the visitor's browser.

controllers/main.py
class FeedbackController(http.Controller):
    @http.route("/feedback/preview", type="http", auth="public")
    def preview(self, comment):
        return request.render("feedback.preview", {"comment": plaintext2html(comment)})

plaintext2html() escapes the text before it turns line breaks into HTML.

Suppressing

body = Markup(body)  # odooscan: ignore[OD703]

See suppressing a finding.