On 5/24/2019 8:39 AM, Christian Couder wrote:
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.
My original thoughts were that we could limit the sparse:path to
local use and disallow it over the wire to the server, but that
distinction is probably not worth the bother. Removing it completely
is fine.
Thanks,
Jeff
