You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2012/07/17 12:14:10 UTC

Re: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c

philip@apache.org writes:

> Author: philip
> Date: Tue Jul 17 10:12:20 2012
> New Revision: 1362434
>
> URL: http://svn.apache.org/viewvc?rev=1362434&view=rev
> Log:
> Allow third party FS modules to be loaded when configured
> with --enable-runtime-module-search.

Until now anyone wanting to write an FS module had a problem: only
modules known to the Subversion project could be loaded and used.
That means that anyone wanting to write their own module had to get a
patch for their module name into the core Subversion code.  Or write
their own loader/server.

I don't think there is any security risk here: I need to write to the
repository fs-type file to get a malicious module to load and if I can
do that it would be far easier to use one of the hook scripts.

We could do the same for RA modules but I have no plans to implement
it.

Disclaimer: at WANdisco we plan to use this feature.

-- 
Philip

Re: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c

Posted by Stefan Sperling <st...@elego.de>.
On Wed, Jul 18, 2012 at 11:45:52AM +0100, Philip Martin wrote:
> Stefan Sperling <st...@elego.de> writes:
> 
> > On Tue, Jul 17, 2012 at 11:14:10AM +0100, Philip Martin wrote:
> >> Disclaimer: at WANdisco we plan to use this feature.
> >
> > Will generally useful FS features developed by Wandisco be fed
> > back to the Subversion project? Or are there plans to develop
> > a competing proprietary backend that only works with Wandisco
> > products and offers generally useful FS features?
> 
> WANdisco is committed to doing FS development as part of the Subversion
> project.  We are interested in all the usual things: ways to store move
> and merge information, different ways to store directories, forward
> history, etc.  We have no plans to develop a proprietary FS backend.

Great! This is what I had hoped and expected.
Thanks for clarifying, Philip!

Re: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c

Posted by Philip Martin <ph...@wandisco.com>.
Stefan Sperling <st...@elego.de> writes:

> On Tue, Jul 17, 2012 at 11:14:10AM +0100, Philip Martin wrote:
>> Disclaimer: at WANdisco we plan to use this feature.
>
> Will generally useful FS features developed by Wandisco be fed
> back to the Subversion project? Or are there plans to develop
> a competing proprietary backend that only works with Wandisco
> products and offers generally useful FS features?

WANdisco is committed to doing FS development as part of the Subversion
project.  We are interested in all the usual things: ways to store move
and merge information, different ways to store directories, forward
history, etc.  We have no plans to develop a proprietary FS backend.

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download

Re: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c

Posted by Stefan Sperling <st...@elego.de>.
On Tue, Jul 17, 2012 at 11:14:10AM +0100, Philip Martin wrote:
> Disclaimer: at WANdisco we plan to use this feature.

Will generally useful FS features developed by Wandisco be fed
back to the Subversion project? Or are there plans to develop
a competing proprietary backend that only works with Wandisco
products and offers generally useful FS features?

I don't mind your change. I'm just curious about what the
Subversion project itself can expect from getting out of this.

RE: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: MARTIN PHILIP [mailto:codematters@ntlworld.com] On Behalf Of
> Philip Martin
> Sent: dinsdag 17 juli 2012 16:07
> To: Ivan Zhakov
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1362434 - in /subversion/trunk: configure.ac
> subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c
> 
> Ivan Zhakov <iv...@visualsvn.com> writes:
> 
> >> If the victim has a world writeable location in the search path the
attacker
> >> could replace any DSO.
> >
> > The attacker cannot replace any DSO, because current directory has
> > lower priority than other locations. So in typical scenarios user is
> > not vulnerable, because DSO is found in other location.
> >
> > So for security reason we should load DSO using absolute path at least
> > on Windows.
> 
> I'm not sure how to get the absolute path.
> 
> Does anyone build on Windows with SVN_USE_DSO set?

I don't think so, as we don't provide a way to enable this via gen-make.py.


We use apr to load libraries and APR loads libraries with the
LOAD_WITH_ALTERED_SEARCH_PATH flag, so Windows starts by checking the
directory in which the current executable is located. So for our own
executables this should be a safe default.

If some other application is using our libraries they should set a good
default policy via SetDllDirectory.
(We could call SetDllDirectory from our own executables, but we should make
sure we don't do it by default for our api users as this might break their
current security model)


For some specific case such as JavaHL (which would run in the java runtime,
where the application dir is something out of our control) we may improve
the situation a bit by providing a full path instead of just a dll name.

But then: Java has already loaded our javahl dll via an unsecure route, so
the front door is already open. No need to lock down the windows :)

(Note: The police recommends to lock the Windows anyway, to make sure
burglars don't feel they have a safe way out. But I don't think this part
applies to software security)

	Bert 


Re: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c

Posted by Philip Martin <ph...@wandisco.com>.
Ivan Zhakov <iv...@visualsvn.com> writes:

>> If the victim has a world writeable location in the search path the attacker
>> could replace any DSO.
>
> The attacker cannot replace any DSO, because current directory has
> lower priority than other locations. So in typical scenarios user is
> not vulnerable, because DSO is found in other location.
>
> So for security reason we should load DSO using absolute path at least
> on Windows.

I'm not sure how to get the absolute path.

Does anyone build on Windows with SVN_USE_DSO set?

-- 
Cerified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download

Re: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Tue, Jul 17, 2012 at 5:21 PM, Philip Martin
<ph...@wandisco.com> wrote:
> Ivan Zhakov <iv...@visualsvn.com> writes:
>
>> On Tue, Jul 17, 2012 at 2:14 PM, Philip Martin
>> <ph...@wandisco.com> wrote:
>>>
>>> philip@apache.org writes:
>>>
>>> > Author: philip
>>> > Date: Tue Jul 17 10:12:20 2012
>>> > New Revision: 1362434
>>> >
>>> > URL: http://svn.apache.org/viewvc?rev=1362434&view=rev
>>> > Log:
>>> > Allow third party FS modules to be loaded when configured
>>> > with --enable-runtime-module-search.
>>>
>>> Until now anyone wanting to write an FS module had a problem: only
>>> modules known to the Subversion project could be loaded and used.
>>> That means that anyone wanting to write their own module had to get a
>>> patch for their module name into the core Subversion code.  Or write
>>> their own loader/server.
>>>
>>> I don't think there is any security risk here: I need to write to the
>>> repository fs-type file to get a malicious module to load and if I can
>>> do that it would be far easier to use one of the hook scripts.
>>>
>> It still possible security issue here. Just image that repository is
>> stored on network share or something. Someone tweaked fs-type and put
>> fake .dll in repository folder. Then another user accesses this
>> repository and gets this dll loaded on his behalf!
>
> To get a DSO loaded it has to go into the library search path.
Problem that on Windows current directory is also library search path [1].

> If the victim has a world writeable location in the search path the attacker
> could replace any DSO.
The attacker cannot replace any DSO, because current directory has
lower priority than other locations. So in typical scenarios user is
not vulnerable, because DSO is found in other location.

So for security reason we should load DSO using absolute path at least
on Windows.


[1] http://msdn.microsoft.com/en-us/library/windows/desktop/ms682586%28v=vs.85%29.aspx#standard_search_order_for_desktop_applications

>> To prevent such issues we should valdiate fs-type to be only file name
>> with only alphanumeric characters. No dots, spaces or slashes. We also
>> should only load DSO module from directory where Subversion installed
>> for better protection.
> That's a good idea.  r1362480.
Great!

-- 
Ivan Zhakov

Re: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c

Posted by Philip Martin <ph...@wandisco.com>.
Ivan Zhakov <iv...@visualsvn.com> writes:

> On Tue, Jul 17, 2012 at 2:14 PM, Philip Martin
> <ph...@wandisco.com> wrote:
>>
>> philip@apache.org writes:
>>
>> > Author: philip
>> > Date: Tue Jul 17 10:12:20 2012
>> > New Revision: 1362434
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1362434&view=rev
>> > Log:
>> > Allow third party FS modules to be loaded when configured
>> > with --enable-runtime-module-search.
>>
>> Until now anyone wanting to write an FS module had a problem: only
>> modules known to the Subversion project could be loaded and used.
>> That means that anyone wanting to write their own module had to get a
>> patch for their module name into the core Subversion code.  Or write
>> their own loader/server.
>>
>> I don't think there is any security risk here: I need to write to the
>> repository fs-type file to get a malicious module to load and if I can
>> do that it would be far easier to use one of the hook scripts.
>>
> It still possible security issue here. Just image that repository is
> stored on network share or something. Someone tweaked fs-type and put
> fake .dll in repository folder. Then another user accesses this
> repository and gets this dll loaded on his behalf!

To get a DSO loaded it has to go into the library search path.  If the
victim has a world writeable location in the search path the attacker
could replace any DSO.

> To prevent such issues we should valdiate fs-type to be only file name
> with only alphanumeric characters. No dots, spaces or slashes. We also
> should only load DSO module from directory where Subversion installed
> for better protection.

That's a good idea.  r1362480.

-- 
Cerified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download

Re: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Tue, Jul 17, 2012 at 2:14 PM, Philip Martin
<ph...@wandisco.com> wrote:
>
> philip@apache.org writes:
>
> > Author: philip
> > Date: Tue Jul 17 10:12:20 2012
> > New Revision: 1362434
> >
> > URL: http://svn.apache.org/viewvc?rev=1362434&view=rev
> > Log:
> > Allow third party FS modules to be loaded when configured
> > with --enable-runtime-module-search.
>
> Until now anyone wanting to write an FS module had a problem: only
> modules known to the Subversion project could be loaded and used.
> That means that anyone wanting to write their own module had to get a
> patch for their module name into the core Subversion code.  Or write
> their own loader/server.
>
> I don't think there is any security risk here: I need to write to the
> repository fs-type file to get a malicious module to load and if I can
> do that it would be far easier to use one of the hook scripts.
>
It still possible security issue here. Just image that repository is
stored on network share or something. Someone tweaked fs-type and put
fake .dll in repository folder. Then another user accesses this
repository and gets this dll loaded on his behalf!

To prevent such issues we should valdiate fs-type to be only file name
with only alphanumeric characters. No dots, spaces or slashes. We also
should only load DSO module from directory where Subversion installed
for better protection.

--
Ivan Zhakov