29 Apr
2010
29 Apr
'10
3:49 p.m.
In the 2.3 timeframe, a serious security bug was fixed. Hibernate treats backslashes
differently from some
database management systems and as a result native SQL can be injected through the
searchDocuments function.
This means members of a wiki can finish an SQL query, also this means that badly written
searchDocuments or
search queries go from small security issues to larger ones.
NOTE: Passwords are hashed (encrypted) so they cannot easily be read from the database.
Who's at risk?
Database systems which treat backslash as an escape character and allow stacked queries
are susceptible to
arbitrary SELECT, INSERT, UPDATE, DELETE, and DROP statements.
These include:
MS-SQL
Postgres
Database systems which treat backslash as an escape character but disallow stacked queries
are susceptible
only to arbitrary SELECT statements.
These include:
MySql
Oracle
Database systems which do not treat backslash as an escape character are not vulnerable
these include:
HSQLDB (default XWiki zip/exe installation)
You can get a small groovy snippet to test your database and see if it supports stacked
queries here:
http://dev.xwiki.org/xwiki/bin/view/Drafts/SecuringXWiki#HMitigationMethods…
What can be done:
#1: XWiki-2.3 and XWiki-2.2.6 Are patched to convert \ to \\ in search queries so
upgrading to them will
negate the threat. You can download them here:
http://www.xwiki.org/xwiki/bin/Main/Download
#2: If you compile your own branch of XWiki and are unable to upgrade, you can integrate
the patch which
was used to fix the problem The patch is here:
http://dev.xwiki.org/xwiki/bin/download/Drafts/SecuringXWiki/XWIKI%2D4755%2…
Since the database controller has changed, you will likely have to port this patch to your
version, what's
important is that api.XWiki.searchDocuments and api.XWiki.search have their input
filtered.
#3: You can and should make sure to log unexpected SQL at the database level. XWiki
doesn't usually use
the backslash character and queries containing backslashes should be logged specially.
Also it is a good
idea to log (or block if possible) any SQL comment syntax. Hibernate does not support
comments and comment
syntax is central to almost all SQL injection.
Caleb
30 Apr
30 Apr
2:28 p.m.
Very simple question:
Instead of manually playing cats & dogs (i.e. escaping backslashes) -
why don't you just use PreparedStatements?
Just a thought...
Rgds
Gregor
--
just because you're paranoid, don't mean they're not after you...
gpgp-fp: 79A84FA526807026795E4209D3B3FE028B3170B2
gpgp-key available
@ http://pgpkeys.pca.dfn.de:11371
@ http://pgp.mit.edu:11371/
skype:rc46fi
3:56 p.m.
We do and users should, but there is a function which allows script authors to construct
queries for document names
so they are allowed to finish an HQL query. If the script author is malicious or if they
don't properly use
prepared statements then SQL can be injected into the HQL.
see XWiki.searchDocuments
http://maven.xwiki.org/site/xwiki-core-parent/xwiki-core/apidocs/com/xpn/xw…
I hope this clears up exactly what the issue is.
Caleb
Gregor Schneider wrote:
Very simple question:
Instead of manually playing cats & dogs (i.e. escaping backslashes) -
why don't you just use PreparedStatements?
Just a thought...
Rgds
Gregor
3 May
3 May
12:07 p.m.
On Apr 30, 2010, at 3:56 PM, Caleb James DeLisle wrote:
We do and users should, but there is a function which
allows script authors to construct queries for document names
so they are allowed to finish an HQL query. If the script author is malicious or if they
don't properly use
prepared statements then SQL can be injected into the HQL.
see XWiki.searchDocuments
http://maven.xwiki.org/site/xwiki-core-parent/xwiki-core/apidocs/com/xpn/xw…
Actually Gregor might be right and we could decide to deprecate this method and recommend
to use one which would take a varargs list of parameters, wdyt?
Thanks
-Vincent
I hope this clears up exactly what the issue is.
Caleb
Gregor Schneider wrote:
> Very simple question:
>
> Instead of manually playing cats & dogs (i.e. escaping backslashes) -
> why don't you just use PreparedStatements?
>
> Just a thought...
>
> Rgds
>
> Gregor
12:16 p.m.
Hi
my request for today: a separate mailinglist for security-advisories so one can subscribe
to only those.
Thank you in advance
Harald Kapper / kapper.net
12:33 p.m.
Vincent Massol wrote:
On Apr 30, 2010, at 3:56 PM, Caleb James DeLisle
wrote:
When there are no user supplied parameters the "bad" method is fine eg:
$searchDocuments("where doc.space='Main'")
Also a script author can still make a mistake with the "good" method and not
parametrize enough eg:
$searchDocuments("where doc.space='" + $userInput + "' and
doc.name=?", [$moreUserInput])
Of course the advisory is considering the possibility of a malicious script author who
could exploit either method.
A good long term answer would be to make the query be written in type safe java where the
code always knows what
needs to be parametrized.
This is an interesting project:
http://source.mysema.com/static/querydsl/latest/reference/html/ch02s04.html
Caleb
We do and users should, but there is a function
which allows script authors to construct queries for document names
so they are allowed to finish an HQL query. If the script author is malicious or if they
don't properly use
prepared statements then SQL can be injected into the HQL.
see XWiki.searchDocuments
http://maven.xwiki.org/site/xwiki-core-parent/xwiki-core/apidocs/com/xpn/xw…
Actually Gregor might be right and we could decide to deprecate this method and recommend
to use one which would take a varargs list of parameters, wdyt?
Thanks
-Vincent
I hope this clears up exactly what the issue is.
Caleb
Gregor Schneider wrote:
> Very simple question:
>
> Instead of manually playing cats & dogs (i.e. escaping backslashes) -
> why don't you just use PreparedStatements?
>
> Just a thought...
>
> Rgds
>
> Gregor
_______________________________________________
users mailing list
users(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/users
12:34 p.m.
On 05/03/2010 12:07 PM, Vincent Massol wrote:
On Apr 30, 2010, at 3:56 PM, Caleb James DeLisle wrote:
That still won't fix the problem, since the query can still hold
non-parameterized code. So, something like this would work:
searchDocuments(" where doc.name like ? and doc.space = 'Main'",
['X%'])
This will only encourage users (devs) to use parameterized queries, but
will still leave the security problem wide open.
We do and users should, but there is a function
which allows script authors to construct queries for document names
so they are allowed to finish an HQL query. If the script author is malicious or if they
don't properly use
prepared statements then SQL can be injected into the HQL.
see XWiki.searchDocuments
http://maven.xwiki.org/site/xwiki-core-parent/xwiki-core/apidocs/com/xpn/xw…
Actually Gregor might be right and we could decide to deprecate this method and recommend
to use one which would take a varargs list of parameters, wdyt?
>
> I hope this clears up exactly what the issue is.
>
> Caleb
>
>
> Gregor Schneider wrote:
>> Very simple question:
>>
>> Instead of manually playing cats& dogs (i.e. escaping backslashes) -
>> why don't you just use PreparedStatements?
>>
>> Just a thought...
>>
>> Rgds
>>
>> Gregor
--
Sergiu Dumitriu
http://purl.org/net/sergiu/
1:14 p.m.
On May 3, 2010, at 12:34 PM, Sergiu Dumitriu wrote:
On 05/03/2010 12:07 PM, Vincent Massol wrote:
Yes but then we could also escape the quote characters... ;)
We would have a backward compat issue to handle though but that's doable with some
security setting for ex.
-Vincent
On Apr 30, 2010, at 3:56 PM, Caleb James DeLisle wrote:
That still won't fix the problem, since the query can still hold
non-parameterized code. So, something like this would work:
searchDocuments(" where doc.name like ? and doc.space = 'Main'",
['X%'])
This will only encourage users (devs) to use parameterized queries, but
will still leave the security problem wide open. We do and users should, but there is a function
which allows script authors to construct queries for document names
so they are allowed to finish an HQL query. If the script author is malicious or if they
don't properly use
prepared statements then SQL can be injected into the HQL.
see XWiki.searchDocuments
http://maven.xwiki.org/site/xwiki-core-parent/xwiki-core/apidocs/com/xpn/xw…
Actually Gregor might be right and we could decide to deprecate this method and recommend
to use one which would take a varargs list of parameters, wdyt? >> I hope this clears up exactly what the issue
is.
>>
>> Caleb
>>
>>
>> Gregor Schneider wrote:
>>> Very simple question:
>>>
>>> Instead of manually playing cats& dogs (i.e. escaping backslashes) -
>>> why don't you just use PreparedStatements?
>>>
>>> Just a thought...
>>>
>>> Rgds
>>>
>>> Gregor
5:07 p.m.
Having read some of the comments here, this is my 0.02€:
I feel that a generic method like seacrhDocuments is way too
dangerous, because, as stated in the comments, malicious SQL can be
inserted easily.
Furthermore, if you try to escape certain characters, you might run
into problems when such a character is part of any XWiki-object.
Therefore, I'd rather have some specialized methods handy, internally
based on PreparedStatements then such a generic problematic method.
I'm aware that there are backward-compatibility-issue, also I'm aware
that such a concept soesn't come as handy as a generic method, but
better be safe than sorry....
Cheers
Gregor
--
just because you're paranoid, don't mean they're not after you...
gpgp-fp: 79A84FA526807026795E4209D3B3FE028B3170B2
gpgp-key available
@ http://pgpkeys.pca.dfn.de:11371
@ http://pgp.mit.edu:11371/
skype:rc46fi
5303
days inactive
5307
days old
8 comments
5 participants
participants (5)
-
Caleb James DeLisle -
Gregor Schneider -
Harald Kapper -
Sergiu Dumitriu -
Vincent Massol