New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ACL check in search_allpages #2609

Merged
merged 1 commit into from Dec 12, 2018

Conversation

Projects
None yet
3 participants
@michitux
Copy link
Collaborator

michitux commented Nov 29, 2018

Due to the changes in 8f34cf3 (first included in the Greebo release), the ACL check in search_allpages was only executed when 'skipacl' has been explicitly set to false. Otherwise, only ACLs for namespaces were checked (unless the sneakyacl option was passed). The documentation states that the default for 'skipacl' is false, so setting it to false shouldn't be necessary.

From all I can see, this does not concern DokuWiki itself as search_allpages is never used without the 'skipacl' option explicitly set to true or false. However, this causes serious security issues in plugins that rely on this ACL check in search_allpages like the include plugin (see dokufreaks/plugin-include#229).

Security: Fix ACL check in search_allpages
Due to the changes in 8f34cf3, the ACL
check in search_allpages was only executed when 'skipacl' has been
explicitly set to false. Otherwise, only ACLs for namespaces were
checked (unless the sneakyacl option was passed). The documentation
states that the default for 'skipacl' is false, so setting it to false
shouldn't be necessary.

From all I can see, this does not concern DokuWiki itself as
search_allpages is never used without the 'skipacl' option explicitly
set to true or false. However, this causes serious security issues in
plugins that rely on this ACL check in search_allpages like the include
plugin.

michitux added a commit to dokufreaks/plugin-include that referenced this pull request Nov 29, 2018

Version 2018-11-29: Security fix for DokuWiki Greebo
If you are using the DokuWiki Greebo release and rely on ACL checks in
the include plugin, apply this change as soon as possible. Note that
this is only an issue with namespace includes, so if you do not use
namespace includes and edits are only allowed for users that have access
to your whole wiki, this does not concern you (but updating is still
recommended).

Note that this is a problem caused by a bug in DokuWiki release Greebo.
A future hotfix release of DokuWiki might fix this, too, see
splitbrain/dokuwiki#2609 for further
information.
@Klap-in

This comment has been minimized.

Copy link
Collaborator

Klap-in commented Nov 29, 2018

There are now a few unit tests for the search. Is it doable to add some to cover this?

@michitux

This comment has been minimized.

Copy link
Collaborator Author

michitux commented Nov 30, 2018

It would certainly be a good idea to add unit tests to test this and that's definitely possible. It's just that I didn't have the time to do this yesterday and I don't know if I'll have time in the next days to work on this.

@splitbrain splitbrain merged commit b058d41 into splitbrain:master Dec 12, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment