Sanitize user input with the USING keyword in PL/pgSQL

Question

This is how I create my search_term:

    IF char_length(search_term) > 0 THEN
        order_by := 'ts_rank_cd(textsearchable_index_col, to_tsquery(''' || search_term || ':*''))+GREATEST(0,(-1*EXTRACT(epoch FROM age(last_edited)/86400))+60)/60 DESC';
        search_term := 'to_tsquery(''' || search_term || ':*'') @@ textsearchable_index_col';
    ELSE
        search_term := 'true';
    END IF;

I am having some trouble with a PLPGSQL function:

    RETURN QUERY EXECUTE '
        SELECT
            *
        FROM
            articles
        WHERE
            $1 AND
            ' || publication_date_query || ' AND
            primary_category LIKE ''' || category_filter || ''' AND
            ' || tags_query || ' AND
            ' || districts_query || ' AND
            ' || capability_query || ' AND
            ' || push_notification_query || ' AND
            ' || distance_query || ' AND
            ' || revision_by || ' AND
            ' || publication_priority_query || ' AND
            ' || status_query || ' AND
            is_template = ' || only_templates || ' AND
            status <> ''DELETED''
        ORDER BY ' || order_by || ' LIMIT 500'
        USING search_term;
    END; $$;

returns ERROR:

argument of AND must be type boolean, not type text at character 64

As opposed to:

        RETURN QUERY EXECUTE '
            SELECT
                *
            FROM
                articles
            WHERE
                ' || search_term || ' AND
                ' || publication_date_query || ' AND
                primary_category LIKE ''' || category_filter || ''' AND
                ' || tags_query || ' AND
                ' || districts_query || ' AND
                ' || capability_query || ' AND
                ' || push_notification_query || ' AND
                ' || distance_query || ' AND
                ' || revision_by || ' AND
                ' || publication_priority_query || ' AND
                ' || status_query || ' AND
                is_template = ' || only_templates || ' AND
                status <> ''DELETED''
            ORDER BY ' || order_by || ' LIMIT 500';
        END; $$;

... which works. Am I missing something?
My goal is to sanitize my user input.

What is `search_term`? If it is something like `column_a = 'some_string'` then it won't work with prepared statement as those can't be used for dynamic SQL. — Łukasz Kamiński, Aug 07 '17 at 09:00
`search_term` is user input a.k.a. an arbitrary string value. — Divide by Zero, Aug 07 '17 at 09:12
Try to change it to `length($1) > 0 AND` if you want to make sure that something is in here. — Adam Tokarski, Aug 07 '17 at 09:59
`search_term` cannot be just *an arbitrary string value*. It must be an *expression* evaluating to a boolean value when executed - or your 2nd code fragment would error out as well. Please show a [SSCCE](http://sscce.org/) and some sample input. Don't dump your whole code fragment with lots of irrelevant noise but missing essential parts. For starters, both your attempts are no good to sanitize user input. Gaping SQL injection holes ... — Erwin Brandstetter, Aug 07 '17 at 11:26
@ErwinBrandstetter you are right. It is not arbitrary. See my Edit on top of my post. I am aware of my vulnerabilities. As a first step, i want to parameterise my data with the USING keyword e.g. `USING search_term;` — Divide by Zero, Aug 07 '17 at 11:30

score 1 · Accepted Answer · answered Aug 07 '17 at 12:47

If some of your input parameters can be NULL or empty and should be ignored in this case, you best build your whole statement dynamically depending on user input - and omit respective WHERE / ORDER BY clauses completely.

The key is to handle NULL and empty string correctly, safely (and elegantly) in the process. For starters, search_term <> '' is a smarter test than char_length(search_term) > 0. See:

Best way to check for "empty or null value"

And you need a firm understanding of PL/pgSQL, or you may be in over your head. Example code for your case:

CREATE OR REPLACE FUNCTION my_func(
         _search_term            text = NULL  -- default value NULL to allow short call
       , _publication_date_query date = NULL 
    -- , more parameters
       )
  RETURNS SETOF articles AS
$func$
DECLARE
   sql       text;
   sql_order text;   -- defaults to NULL

BEGIN
   sql := concat_ws(' AND '
    ,'SELECT * FROM articles WHERE status <> ''DELETED'''  -- first WHERE clause is immutable
    , CASE WHEN _search_term <> ''            THEN '$1 @@ textsearchable_index_col' END  -- ELSE NULL is implicit
    , CASE WHEN _publication_date_query <> '' THEN 'publication_date > $2'          END  -- or similar ...
 -- , more more parameters
   );

   IF search_term <> '' THEN  -- note use of $1!
      sql_order  := 'ORDER BY ts_rank_cd(textsearchable_index_col, $1) + GREATEST(0,(-1*EXTRACT(epoch FROM age(last_edited)/86400))+60)/60 DESC';
   END IF;

   RETURN QUERY EXECUTE concat_ws(' ', sql, sql_order, 'LIMIT 500')
   USING  to_tsquery(_search_term || ':*')  -- $1  -- prepare ts_query once here!
        , _publication_date_query           -- $2  -- order of params must match!
     -- , more parameters
   ;

END
$func$  LANGUAGE plpgsql;

I added default values for function parameters, so you can omit params that don't apply in the call. Like:

SELECT * FROM my_func(_publication_date_query => '2016-01-01');

More:

Note the strategic use of concat_ws(). See:

How to concatenate columns in a Postgres SELECT?

Here is a related answer with lots of explanation:

Test for null in function with varying parameters

Sanitize user input with the USING keyword in PL/pgSQL

1 Answers1

Linked