SQL injection with Jinja 2 templates: how to escape values?

[d9k]

(Assuming we have

FEATURE_FLAGS = {
   # . . . . .
  "ENABLE_TEMPLATE_PROCESSING": True
}

)

For example, I have a filter

{% set filter_city = filter_values("example_filter_city_name", remove_filter = True) %}

{% if filter_city | length %}
  example_city_name IN  (
    {{ "'" + "','".join(filter_city) + "'" }}
  )
  OR example_city_name IS NULL
{% else %}
  TRUE
{% endif %}

How can I escape SQL values in {{ "'" + "','".join(filter_city) + "'" }} expression to make this filter template safe?

See also:

• Superset doc on sql templating.
• Custom SQL queries with filters #1996 (comment)
• Adds a new macro to allow getting filter values easily #5547

[d9k]

As a temporary poor man's solution I've added custom functions for jinja2 templates to escape PrestoDb strings. These functions use replacement of single quote with CHR(39) concatenation:

# >> escape_presto_string("Dr's")
# "Dr' || CHR(39) || 's"
def escape_presto_string(s = ''):
  return s.replace("'", "' || CHR(39) || '")

# >> escape_presto_strings(["Dr's", 't'])
# ["Dr' || CHR(39) || 's", 't']
def escape_presto_strings(ar = []):
  return list(map(escape_presto_string, ar))

# >> escape_presto_strings_in(["Dr's", 't'])
# "'Dr' || CHR(39) || 's','t'"
def escape_presto_strings_in(ar = []):
  escaped_ar = escape_presto_strings(ar)
  if (len(escaped_ar) == 0):
    return ""
  return "'" + "','".join(escaped_ar) + "'"

JINJA_CONTEXT_ADDONS = {
    "escape_presto_string": escape_presto_string,
    "escape_presto_strings": escape_presto_strings,
    "escape_presto_strings_in": escape_presto_strings_in,
}

[d9k]

So code above becomes:

{% set filter_city = filter_values("example_filter_city_name", remove_filter = True) %}

{% if filter_city | length %}
  example_city_name IN  ({{ escape_presto_strings_in(filter_city) }})
  OR example_city_name IS NULL
{% else %}
  TRUE
{% endif %}

[d9k]

I want to improve escape_presto_string() function by calling data source "native" escape function.
How to get current sql alchemy connection in escape_presto_string() and which methods should I call to escape string value?

[andrey-mikhailov]

I think it's good practise to deny requests with non printable characters and check field length.
For example, mysql-real-escape-string function escapes \x00, \n, \r, , ', " and \x1a characters but people usually don't have

[d9k]

@andrey-mikhailov don't have what?

mysql-real-escape-string function

Superset uses python, not php

[Aalbedon]

I know it's an older topic, but since this didn't provide the answer and I've found it since, I figured that for future people looking for the same solution I figured I'd post what I found.
You can escape Jinja parameters/variables using standard Jinja syntax (see: https://jinja.palletsprojects.com/en/3.1.x/templates/#html-escaping).
Simply add |e to the end of the variable for which you want to escape the characters.

Sample code that allows you to search partial matches of a list of values provided in a dashboard filter, but escapes the unsafe characters like the single quote:

select 
  t.order_id as "Order ID",
  t.status_desc as "Status"
from my_table t
where
(1=1)
        {% if (filter_values('Status') is defined) and filter_values('Status')|length > 0 %}
            AND ({% for value in filter_values('Status', remove_filter=True) %}
                {% if value %} upper(t.status_desc) LIKE upper('{{ "%" + value|e + "%" }}') {% endif %}{% if not loop.last %} OR {% endif %}
            {% endfor %} 
            )
        {% endif %}

Test SQL injection by providing this in a dashboard filter:
ALICE') or t.status_desc like upper('BOB

[seiyab]

Thanks, it looks to work well to me for some malcious inputs.
I'm worrying about its completeness. According to the doc, |e filter looks HTML escaping. Is it enough for SQL?

[seiyab]

It should be very important point to use Jinja templates. I consider that the Superset doc should mention that using Jinja Template is vulnerable to SQL injection so we need manual escaping or {% autoescape True %}.

[Aalbedon]

I completely agree with you.
Also, I am not a security or pen test expert, just speaking from the limited exposure to this I've had.

I recall having read an article somewhere that talks about creating a custom function and adding it to Superset so that it can clean inputs.
That's not an option for everyone.
With how vulnerable Jinja templating is to SQL injection, there probably should be indeed an option to enable or disable it/apply it.

In any case, HTML escaping just happens to escape the single quote, which is one of the main offenders in SQL injection.
Jinja templating can still be applied safely if all you're using it for is generating SQL code blocks that do NOT use a Jinja templating variable inline.
Another thing to consider is using low permission accounts for anything that connects to a Superset datasource (or Trino Catalog if that's what you're building your datasets on).