plpgsql function SQL injection vulnerability?

[Unto Sten]

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

[George Neuner]

On Thu, 25 Oct 2018 17:11:17 -0000 (UTC), sten...@gmail.com (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

[Laurenz Albe]

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.

[Unto Sten]

Laurenz Albe <lau...@nospam.pn> 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

[Unto Sten]

George Neuner <gneu...@comcast.net> 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