1. Forum
  2. Usenet
  3. COMP.DATABASES.POSTGRESQL

  • plpgsql function SQL injection vulnerability?

    From Unto Sten@21:1/5 to All on Thu Oct 25 17:11:17 2018
    Hello!

    I have a question that is probably easy for the
    PostgreSQL experts. Consider a simple function:

    ###################################

    CREATE OR REPLACE FUNCTION search_for_address(re TEXT)
    RETURNS TABLE(line VARCHAR) AS $$
    BEGIN
    RETURN QUERY SELECT k.line FROM kdata k WHERE k.line ~* re ORDER BY k.line ASC LIMIT 100;
    END;
    $$ LANGUAGE plpgsql SECURITY DEFINER;

    ###################################

    Is this function vulnerable to SQL injection attacks
    via input 're TEXT' or does the PG parser prevent it
    in these plpgsql functions?

    To be safe, I do input validation before calling
    search_for_address(re TEXT) but I would like to
    know the truth here.

    If the function is vulnerable, could you please provide
    an exact string to prove it? Thanks.

    I have tried to attack it, but my attempts failed.

    Best regards,
    Unto Sten

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From George Neuner@21:1/5 to Sten on Fri Oct 26 00:47:35 2018
    On Thu, 25 Oct 2018 17:11:17 -0000 (UTC), [email protected] (Unto
    Sten) wrote:

    I have a question that is probably easy for the
    PostgreSQL experts. Consider a simple function:

    ###################################

    CREATE OR REPLACE FUNCTION search_for_address(re TEXT)
    RETURNS TABLE(line VARCHAR) AS $$
    BEGIN
    RETURN QUERY SELECT k.line FROM kdata k WHERE k.line ~* re ORDER BY k.line ASC LIMIT 100;
    END;
    $$ LANGUAGE plpgsql SECURITY DEFINER;

    ###################################

    Is this function vulnerable to SQL injection attacks
    via input 're TEXT' or does the PG parser prevent it
    in these plpgsql functions?

    This particular use is safe: the 're' argument to the function is
    passed as a parameter to the ~* regex operator in a statc query ...
    the contents of the 're' string can't escape the operator's scope.

    Static queries that take parameters mostly are immune to injection. It
    is possible to inject bogus data which will cause the query to fail or
    return the wrong results ... but the query using parameters can't be
    rewritten so as to do something completely different.

    Injection is much more a concern with dynamic queries: e.g., the query
    is provided as a function argument, or is constructed by concatenating
    strings that include function arguments, and then is run using
    EXECUTE. Sometimes you have no choice[*] but most queries can be
    written safely using parameters. Dynamic queries more often are the
    result of programmer laziness than of real necessity.


    [*] some things can't be paramterized in a query: e.g., schema and
    table names, operators, filtering conjuctives (AND/OR), etc.


    To be safe, I do input validation before calling
    search_for_address(re TEXT) but I would like to
    know the truth here.

    If the function is vulnerable, could you please provide
    an exact string to prove it? Thanks.

    Assuming there is a good reason for the function to be a security
    definer, it looks fine.

    George

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Laurenz Albe@21:1/5 to George Neuner on Fri Oct 26 12:47:53 2018
    On Fri, 26 Oct 2018 00:47:35 -0400, George Neuner wrote:

    CREATE OR REPLACE FUNCTION search_for_address(re TEXT)
    RETURNS TABLE(line VARCHAR) AS $$
    BEGIN
    RETURN QUERY SELECT k.line FROM kdata k WHERE k.line ~* re ORDER
    BY
    k.line ASC LIMIT 100;
    END;
    $$ LANGUAGE plpgsql SECURITY DEFINER;

    Is this function vulnerable to SQL injection attacks via input 're
    TEXT'
    or does the PG parser prevent it in these plpgsql functions?

    This particular use is safe: the 're' argument to the function is passed
    as a parameter to the ~* regex operator in a statc query ...
    the contents of the 're' string can't escape the operator's scope.

    The function may be safe from SQL injection, but it is vulnerable to
    privilege escalation attacks.

    The attacker could define a ~* operator in "his" schema, set search_path
    to that schema and call the function to execute arbitrary code with
    elevated privileges.

    That's why you should always define SECURITY DEFINER functions with
    SET search_path=pg_catalog
    and schema qualify all access to objects in other schemas.

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Unto Sten@21:1/5 to Laurenz Albe on Fri Oct 26 19:06:15 2018
    Laurenz Albe <[email protected]> wrote:
    The function may be safe from SQL injection, but it is vulnerable to privilege escalation attacks.

    The attacker could define a ~* operator in "his" schema, set search_path
    to that schema and call the function to execute arbitrary code with
    elevated privileges.

    That's why you should always define SECURITY DEFINER functions with
    SET search_path=pg_catalog
    and schema qualify all access to objects in other schemas.

    Now that information is very important. Thanks a lot for this tip!

    This code is not in production yet, and I will make sure to implement
    your additions. Whoa, I am glad I asked!

    Best regards,
    Unto Sten

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Unto Sten@21:1/5 to George Neuner on Fri Oct 26 19:08:23 2018
    George Neuner <[email protected]> wrote:
    This particular use is safe: the 're' argument to the function is
    passed as a parameter to the ~* regex operator in a statc query ...
    the contents of the 're' string can't escape the operator's scope.

    Okay, thanks, I suspected that.

    Static queries that take parameters mostly are immune to injection. It
    is possible to inject bogus data which will cause the query to fail or
    return the wrong results ... but the query using parameters can't be rewritten so as to do something completely different.

    Understood.

    Injection is much more a concern with dynamic queries: e.g., the query
    is provided as a function argument, or is constructed by concatenating strings that include function arguments, and then is run using
    EXECUTE. Sometimes you have no choice[*] but most queries can be
    written safely using parameters. Dynamic queries more often are the
    result of programmer laziness than of real necessity.

    Thanks for the info and have a great weekend!

    Best regards,
    Unto Sten

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)