Skip to content
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

Private pages can be overriden by URL hacking #44

Open
dplanella opened this issue Mar 28, 2017 · 6 comments
Open

Private pages can be overriden by URL hacking #44

dplanella opened this issue Mar 28, 2017 · 6 comments

Comments

@dplanella
Copy link

Thanks for your great work on Omnigollum.

While using it, I've just noticed that if you specify protected_routes for Omnigollum in Gollum's config.rb as such:

  :protected_routes => [
    '/private/*',
    '/private'],

Then if you go to e.g. https://mywiki.com/private the authorization prompt is shown as expected. However, there seems to be an easy way to override this:

I'm by no means a Ruby developer, so I'm not too sure what's going on behind the scenes. protected_routes is processed here: https://github.com/arr2036/omnigollum/blob/master/lib/omnigollum.rb#L311

Something I tried was to modify the route, so that it's converted to downcase. While that works for that particular case, then unauthenticated users cannot access the open parts of the site that use capital letters, e.g. mywiki.com/Home. So no, the workaround does not quite work:

# Pre-empt protected routes
      options[:protected_routes].each {|route| app.before(route.downcase!) {user_auth unless user_authed?}}

I'm sure there are better and cleverer ways to fix this.

Thanks.

@dplanella dplanella changed the title Private routes can be overriden by URL hacking Private pages can be overriden by URL hacking Mar 28, 2017
@ctreffe
Copy link

ctreffe commented Jul 10, 2018

I just stumbled upon this issue while setting up gollum with omnigollum. Also having the described problem, I finally was able to solve this using regular expressions in the config.rb:

:protected_routes => [
    /\/[Pp][Rr][Ii][Vv][Aa][Tt][Ee]\/.*/,
    /\/[Pp][Rr][Ii][Vv][Aa][Tt][Ee]/
]

I'm sure this solution can easily be improved, but being no ruby coder, this was the best I could come up with. Anyway, maybe this helps.

@mvdkleijn
Copy link

While it is good that @ctreffe found a workaround, this is basically a (fairly major) security issue in omnigollum code. Any chance this is going to get fixed?

@arr2036
Copy link
Owner

arr2036 commented Aug 4, 2020

@mvdkleijn It's not a security flaw in omnigollum.

There is no way to turn off case sensitive route matching in Sinatra (that I could find, please correct me if I'm wrong). So you have two options. Perform URL normalisation on the web server, or use case insensitive regexes demonstrated above. I wouldn't necessarily write them like that though.

Assuming ruby is smart enough to not treat / as a quoting character using the alternative regex syntax, I'd expect %r{^/private/.*}i to work...

@arr2036
Copy link
Owner

arr2036 commented Aug 4, 2020

If you want to confirm it does, I'll add some notes to the readme. Note that the default endpoints that omnigollum protects ARE case sensitive, and so the default globbing style patterns work fine.

@mvdkleijn
Copy link

@mvdkleijn It's not a security flaw in omnigollum.

I stand corrected. It is a "feature" of Sinatra indeed. 😄

@dometto
Copy link

dometto commented Sep 6, 2020

I think the case-sensitivity issue is no longer a live one since gollum 5.x, since we're now matching all routes (not just 'special' routes such as /edit, but also the paths to documents) case sensitively.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants