On Fri, May 24, 2019 at 2:21 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > > > On Fri, May 24 2019, Christian Couder wrote: > > > If someone wants to use as a filter a sparse file that is in the > > repository, something like "--filter=sparse:oid=<ref>:<path>" > > already works. > > > > So 'sparse:path' is only interesting if the sparse file is not in > > the repository. In this case though the current implementation has > > a big security issue, as it makes it possible to ask the server to > > read any file, like for example /etc/password, and to explore the > > filesystem, as well as individual lines of files. > > Removing this is a good idea. > > Just to clarify, what was the attack surface in practice? We pass this > to add_excludes_from_file_to_list(), are there cases where it'll spew > out parse errors/warnings that allow you to extract content from such a > file? Peff provided an example script in: https://public-inbox.org/git/20181108050755.GA32158@xxxxxxxxxxxxxxxxxxxxx/ > Or does it amount to a DoS vector by pointing to some huge (binary?) > file on-disk, or a security issue where you could via the error or > timings discover whether the file exists or not, something else? > > I wonder if server operators need to be paranoid about the DoS from the > issue with <oid>:<path> noted int/perf/p0100-globbing.sh which this is > presumably vulnerable to, i.e. someone with repository write access > uploading pathological patterns. That might be particularly annoying for > e.g. GitHub where the fork network's object storage is shared. In general servers should limit the git processes they launch, but yeah it might be interesting to look at that too.
