Skip to content

Fix traversal bug - #306

Merged
ndmitchell merged 3 commits into
ndmitchell:masterfrom
gaverhae:fix-traversal-bug
May 30, 2019
Merged

Fix traversal bug#306
ndmitchell merged 3 commits into
ndmitchell:masterfrom
gaverhae:fix-traversal-bug

Conversation

@gaverhae

Copy link
Copy Markdown
Contributor

There is a bug in the current path handling code that allows an attacker with direct access to the hoogle server to force it to return files it has access to. This does not seem to work through an nginx proxy such as the setup on hoogle.haskell.org.

Repro on current master:

$ git clone git@github.com:ndmitchell/hoogle.git
Cloning into 'hoogle'...
[...]
$ cd hoogle
$ git rev-parse HEAD
4fc77219521055ac96eca9908b4fb9a63ec9a0c5
$ stack init
[...]
* Matches nightly-2019-05-27
Selected resolver: nightly-2019-05-27
Initialising configuration using resolver: nightly-2019-05-27
Total number of user packages considered: 1
Writing configuration to file: stack.yaml
All done.
$ stack build
[...]
$ stack exec -- hoogle generate --local
Starting generate
Reading ghc-pkg... 0.12s
[11/143] attoparsec... 0.06s
[16/143] basement... 0.23s
[35/143] cryptonite... 0.10s
[52/143] ghc... 2.40s
[76/143] memory... 0.02s
[116/143] transformers... 0.08s
[141/143] zlib... 0.03s
Packages missing documentation: hoogle rts
Found 134 warnings when processing items
Reordering items... 0.02s
Writing tags... 0.15s
Writing names... 0.14s
Writing types... 0.89s
Took 10.31s
$ echo hi > hello
$ stack exec -- hoogle server --port=8080 --local > log.txt &
[1] 35053
$ curl localhost:8080/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/hello
hi
$

This PR tries to solve that problem by bridging the existing gap between URI parsing for replays (which does attempt to prevent this kind of issue) and for the server, which for some reason use completely different code paths.

@ndmitchell

Copy link
Copy Markdown
Owner

Thanks for the patch. I'm going to need to study this one in detail so will review Thursday.

@gaverhaegaverhae mentioned this pull request May 28, 2019
@ndmitchell
ndmitchell merged commit e8e9b66 into ndmitchell:masterMay 30, 2019
@ndmitchell

Copy link
Copy Markdown
Owner

Very nice work - thanks a lot!

achirkin pushed a commit to achirkin/hoogle that referenced this pull request Nov 26, 2020
cocreature added a commit to cocreature/hoogle that referenced this pull request Apr 8, 2021
This fixes another directory traversal for paths of the form
http://localhost:8080/..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2fetc/nixos/configuration.nix.
Also added a bunch of tests for the validation logic since I really
don’t trust it given that this is the second issue after the first one
got fixed almost two years ago in ndmitchell#306.
I’m also starting to wonder if hoogle should have an option to just
disable the generic file server completely. In anything some type of
production setup, serving the required static assets via nginx seems
like a much more sensible solution than trying to get this right in
hoogle.
@cocreaturecocreature mentioned this pull request Apr 8, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@gaverhae@ndmitchell