[CKAN-Security] Two more security issues

Tue Aug 11 11:41:32 UTC 2015

David (Read) forgot to include the list and David (Miller) on this message:

On 10 August 2015 at 16:54, David Read <david.read at hackneyworkshop.com>
wrote:

> David,
>
> I love the line about finding a "scarier exploit"! Datastore is a
> relatively thin wrapper around postgres so there is only a few pages
> of code.
>
> > I'm going to assume that the API endpoint that exposes SQL as a
> parameter has been really really thoroughly pentested already though ? ;)
>
> I'm not clear that anyone professionally qualified has tested this
> extension before - only others. There aren't many deployments of
> Datastore yet tbh - I suspect just you, Natural History Museum and HDX
> (you could check with them directly). So it's great that you're
> contributing the results of your test.
>
> As you suggest, if you could provide the report publicly (once the
> concerns have been allayed or bugs fixed), that would be another
> useful contribution for the community's benefit.
>
> Dave
>
> On 10 August 2015 at 16:18, David Miller <david at openhealthcare.org.uk>
> wrote:
> > Hi Adria,
> >
> > I can't currently see the Gitlab issues - am I trustworthy enough for
> > access? :)
> >
> > Re 1. Broadly, I'm happy to tell our pentesters "Can you come up with a
> > scarier exploit please" and see if they come up with anything.
> >
> > I'm going to assume that the API endpoint that exposes SQL as a parameter
> > has been really really thoroughly pentested already though ? ;)
> >
> > Are details of the results of such reports available anywhere?
> >
> > Re 2. the pentesters certainly thought that you could get it to execute -
> > they sent me a screenshot of an alert box - they've not left the proof of
> > concept on our instance though, I can ask them what the exact parameter
> was.
> >
> > HTH
> >
> > Best,
> >
> > David Miller
> > Open Health Care
> >
> > On Mon, 10 Aug 2015 at 12:46 Adrià Mercader <adria.mercader at okfn.org>
> wrote:
> >>
> >> 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
> >>
> >> The current user and the current database they might have got using #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
> >>
> >>
> >
> > _______________________________________________
> > CKAN security
> > https://lists.okfn.org/mailman/listinfo/security
> >
> https://lists.okfn.org/mailman/options/security/david.read%40hackneyworkshop.com
> >
> > Repo: https://github.com/ckan/ckan-security
> _______________________________________________
> 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/20150811/a27d58d0/attachment-0001.html>