On Tue, Apr 10, 2007 at 10:27:51AM -0700, Eric Gillespie wrote:
> > - Does mod_dav restrict activity names to a filename-safe subset?
> > What's to stop someone performing a MKACTIVITY/MERGE against an activity
> > called '../../../../etc/passwd', for example (or 'NUL' or 'AUX'...)?
>
> If i understand correctly, yes. See repos.c where it parses it
> out of request URLs, which look like
>
> /test/!svn/act/56f63f40-e12e-11db-b643-efea7bcec873
>
Okay, so I'm willing to believe that the combination of our parsing and
APR's uri functions prevent someone from constructing an activity
containing a slash (maybe; we parse out the activity ID in quite a few
places) and probably also a '..' path component, but I see nothing that
prevents me from specifying an activity called AUX, for example. If we
could (i.e. were allowed to - I don't know whether we can) verify that
activity names matched a GUID, that'd be fine, but I don't see that we
do that.
IOW: This code scares me from a security POV. Could someone else who's
really familiar with DAV look at it and tell me its fine, or, failing
that, can we take a closer look and prove it to our own satisfaction.
We cannot let clients directly specify the names of files to be created
on the server without either restricting them to a subset of possible
names, filtering out bad names, or escaping them so that they're known
to be non-bad.
> > - Are we happy to decide that the activities are tied to a specific
> > mod_dav_svn version? i.e. we don't need to provide for compatibility
> > between mod_dav_svn 1.5 and activities created with mod_dav_svn 1.4.
>
> I say so. They are, after all, transitory.
>
Sure, in almost all cases, though we also support (in the API)
re-opening an old transaction and continuing to work with it, and so it
would (e.g.) probably not be acceptable for the filesystem to trash the
current set of transactions on an upgrade.
However, I think it's okay for mod_dav_svn to trash its activity ->
transaction name map, because we don't make the same promises for the
DAV API. (By which I really do mean the _DAV_ API, not 'the RA API as
implemented by ra_dav'). I just want to make sure others feel the same
way.
Regards,
Malcolm
- application/pgp-signature attachment: stored
Received on Tue Apr 10 23:57:47 2007