omnigollum icon indicating copy to clipboard operation
omnigollum copied to clipboard

Private pages can be overriden by URL hacking

Open dplanella opened this issue 7 years ago • 6 comments

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:

  • Try to go to https://mywiki.com/Private (notice the capital 'P'), and voila! you can now view the supposedly private page.

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 avatar Mar 28 '17 14:03 dplanella

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.

ctreffe avatar Jul 10 '18 00:07 ctreffe

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?

mvdkleijn avatar Aug 04 '20 14:08 mvdkleijn

@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 avatar Aug 04 '20 17:08 arr2036

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.

arr2036 avatar Aug 04 '20 17:08 arr2036

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

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

mvdkleijn avatar Aug 04 '20 21:08 mvdkleijn

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.

dometto avatar Sep 06 '20 14:09 dometto