eXist-db / exist

eXist Native XML Database and Application Platform
https://exist-db.org
GNU Lesser General Public License v2.1
429 stars 179 forks source link

Security issue in Package installation #1877

Open adamretter opened 6 years ago

adamretter commented 6 years ago

When installing an EXPath Package to eXist-db, the deployment script changes the permissions on all Collections and Documents in the package when they are stored into the database.

See line https://github.com/eXist-db/exist/blob/develop/src/org/exist/repo/Deployment.java#L892

Basically the deployment sets these mode bits:

  1. owner +read
  2. group +write
  3. other +execute

I find this very uncomfortable, in particular:

  1. group +write which allows anyone in the deploying users group to modify the files in the package.
  2. other +execute which allows any XQuery in the package to be executed by any user. This seems like a bad plan, as an app package may have its own security requirements and/or login processes.

I think this really has to be fixed! I would particularly like to hear comments from @wolfgangmm @dizzzz @duncdrum and @joewiz.

duncdrum commented 6 years ago

Phew I smell a big can of worms incoming. Just to be clear, is this for an app with an app specific user generated during install?

  1. owner ends up being +rwe trough this but we could make this explicit
  2. db-group write make sense to me, although we should check if this propagates to a user-group that is generated by the install script. e.g. dba installs my-app with app-user created on install, is there a app-user group with write access after post-install.xql finished running?
  3. for library packages it seems necessary to have read and execute permissions for all

Since potential changes to this will change how apps function, I would add this to the 5.0.0 milestone.

In the end I feel that this behaviour is actually what most users would end up wanting for their apps, we could make this more configurable via the repo.xml during install.

This will also need-documentation see eXist-db/documentation#164

duncdrum commented 4 years ago

As discussed in the community call, a more restrictive pattern would be to only have the other +execute on pre-install, post-install, and cleanup. If a package requires more wide-ranging permissions these should be set via pre-, or post-install.

Since existing apps might break, this is now a 6.0.0 feature. A more wide ranging review of permissions during app install will likely have to await a major upgrade to expath specs.

The above is not a real solution but a quick fix that helps folks adapt, while reducing attack surface.