[CKAN-Security] Two more security issues

[Adrià Mercader]

I've added my comments to the security repo

Tl;DR: they look superscary in a pentest report but the actual effects on
CKAN don't seem to be as severe.

In any case there might be something that I'm missing and we'll discuss
them among all core devs tomorrow on the dev meeting. I'll keep you posted
of any decision.

Here are my comments, if you can provide additional details that would be
great:

1. SQL Injection

The datastore_search_sql endpoint obviously accepts an SQL string as
parameter. Is this the concern raised or is there a specific SQL statement
that they think presents a threat?

This is the statement on the screenshot (in case someone doesn't want to
type it again!):

(SELECT (CHR(113)||CHR(106)||CHR(107)||CHR(100)||CHR(113))||(SELECT
(CASE WHEN (2110=2110) THEN 1 ELSE 0
END))::text||(CHR(113)||CHR(97)||CHR(106)||CHR(104)||CHR(113)))

As far as I can tell, as scary as this looks like is only generating a
string from the character codes supplied, with a 1 in the middle for good
measure:

http://demo.ckan.org/api/action/datastore_search_sql?sql=%28SELECT%20%28CHR%28113%29||CHR%28106%29||CHR%28107%29||CHR%28100%29||CHR%28113%29%29||%28SELECT%20%28CASE%20WHEN%20%282110=2110%29%20THEN%201%20ELSE%200%20END%29%29::text||%28CHR%28113%29||CHR%2897%29||CHR%28106%29||CHR%28104%29||CHR%28113%29%29%29
<http://demo.ckan.org/api/action/datastore_search_sql?sql=%28SELECT%20%28CHR%28113%29%7C%7CCHR%28106%29%7C%7CCHR%28107%29%7C%7CCHR%28100%29%7C%7CCHR%28113%29%29%7C%7C%28SELECT%20%28CASE%20WHEN%20%282110=2110%29%20THEN%201%20ELSE%200%20END%29%29::text%7C%7C%28CHR%28113%29%7C%7CCHR%2897%29%7C%7CCHR%28106%29%7C%7CCHR%28104%29%7C%7CCHR%28113%29%29%29>

The current user and the current database they might have got using #10
<https://gitlab.com/ckan/ckan-security/issues/10>

#10 refers to the ability to run internal functions like current_database()
(
http://demo.ckan.org/api/action/datastore_search_sql?sql=select%20current_database%28%29
)

The consensus among devs is that although it would be nice to prevent these
calls they are actually not giving away that much details that eg couldn't
find consulted on publicly available deployment scripts on most instances.
And most importantly, the datastore_search_sql endpoint runs on a
connection that uses a read-only user, so even if people could craft an
statement that affected the db they wouldn't have the permissions to run it.

Of course, all this needs to be explained somewhere in the docs or in a
statement.




2. XSS on description group

It does end up like <script>alert(1)</script> but AFAICT it doesn't get
interpreted?

http://i.imgur.com/YwAldvR.png



On 7 August 2015 at 11:43, Ross Jones <ross at servercode.co.uk> wrote:

> Hi,
>
> David (CC'd) has kindly allowed me to check out the pentest that was
> performed on his/our CKAN instance, and so I'm adding the two top-level
> items so that we can discuss them - I'll add them to the gitlab issues list
> today so that we can track them.
>
> 1. SQL Injection
>
> Problem: It was found that one of the API pages used in your application
> accepts SQL statements as a GET parameter which can allow an attacker to
> run arbitrary database queries (we sort of knew this ;) ).
>
> Result: SQLMap found it was possibly (with a rather convoluted query) to
> obtain the name of the database, and the name of the user that is
> connecting to the database.  SQLMap does this as a PoC, but shows that it
> is possible to bypass any checking that is performed to obtain information
> that isn't really public.
>
> 2. XSS on description field in group
>
> Problem: It was found that when the application displays descriptions of
> collections which may be created by users, it does not correctly filter
> special characters such as HTML tags (<).
>
> Result: This vulnerability could enable users who unwittingly click on a
> malicious link to leak private information to attackers. This information
> typically includes cookies which provide access to the user’s account and
> session.
>
> I'm pretty worried by 1, so will be adding a PR soon to allow admins to
> disable this functionality if they wish.  I haven't checked this specific
> code, is anyone confident that it *can* be fixed to be secure? Seems like
> the type of code that everyone will be chasing down edge cases on forever.
>
> Cheers
>
> Ross.
> _______________________________________________
> CKAN security
> https://lists.okfn.org/mailman/listinfo/security
> https://lists.okfn.org/mailman/options/security/adria.mercader%40okfn.org
>
> Repo: https://github.com/ckan/ckan-security
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.okfn.org/mailman/private/security/attachments/20150810/31ba83f1/attachment-0001.html>